Update multiple unittests from CPython 3.10.5#3910
Conversation
youknowone
force-pushed
the
update-container-tests
branch
from
b8fa3e2 to
ebc7b57
Compare
| def test_imul(self): | ||
| super().test_imul() |
There was a problem hiding this comment.
Can the ones around here that no longer have the @unittest.expectedFailure decorators on them be removed? @frank-king left these behind without realizing why I put them there.
There was a problem hiding this comment.
@fanninpm you don't need to mention someone unless we actually need their attention. In this case, I think referring commit or pr url can be helpful, but not about the author.
| # TODO: RUSTPYTHON | ||
| @unittest.expectedFailure | ||
| def test_contains_fake(self): | ||
| super().test_contains_fake() | ||
|
|
||
| # TODO: RUSTPYTHON | ||
| @unittest.expectedFailure | ||
| def test_count(self): | ||
| super().test_count() | ||
|
|
||
| # TODO: RUSTPYTHON | ||
| @unittest.expectedFailure | ||
| def test_index(self): | ||
| super().test_index() |
There was a problem hiding this comment.
When I add things like this, I'd like to direct contributors against leaving the last two lines of this pattern behind when they un-mark these tests.
| # TODO: RUSTPYTHON | ||
| @unittest.expectedFailure | ||
| def test_count(self): | ||
| super().test_count() | ||
|
|
||
| # TODO: RUSTPYTHON | ||
| @unittest.expectedFailure | ||
| def test_contains_fake(self): | ||
| super().test_contains_fake() |
There was a problem hiding this comment.
When I add things like this, I'd like to direct contributors against leaving the last two lines of this pattern behind when they un-mark these tests.
| # TODO: RUSTPYTHON | ||
| @unittest.expectedFailure | ||
| def test_contains_fake(self): | ||
| super().test_contains_fake() | ||
|
|
||
| # TODO: RUSTPYTHON | ||
| @unittest.expectedFailure | ||
| def test_count(self): | ||
| super().test_count() |
There was a problem hiding this comment.
When I add things like this, I'd like to direct contributors against leaving the last two lines of this pattern behind when they un-mark these tests.
youknowone
force-pushed
the
update-container-tests
branch
from
ebc7b57 to
a5dac57
Compare
youknowone
force-pushed
the
update-container-tests
branch
from
a5dac57 to
6842f9b
Compare
youknowone
changed the title
|
@fanninpm I added additional comments for new methods from rustpython and added 4 more commits |
youknowone
force-pushed
the
update-container-tests
branch
from
6842f9b to
6729417
Compare
| # TODO: RUSTPYTHON | ||
| @unittest.expectedFailure | ||
| def test_index(self): | ||
| super().test_index() |
There was a problem hiding this comment.
In the future, classes dependent on this test may start unexpectedly passing. Will they unexpectedly pass all at once or one at a time?
There was a problem hiding this comment.
Tests are supportive code. They will be handled when they actually be solved. We can spend cost when we actually need it.
The answer is I don't know. Sometimes they passes at once, sometimes not. It depends on problems and solutions. I didn't look in the details.
fanninpm
left a comment
fanninpm
left a comment
There was a problem hiding this comment.
Just a few minor cleanup notes.
| def test_init(self): | ||
| super().test_init() |
There was a problem hiding this comment.
The test_init method in list_tests.CommonTest is no longer marked. I don't think this needs to be here.
There was a problem hiding this comment.
@fanninpm we expect code review is about changed code, instead of arbitrary code around changed code.
I think that change is independent from this PR. If you want me to fix it, I will do it in separated PR.
| def test_count(self): # XXX: RUSTPYTHON; the method also need to be removed when done | ||
| super().test_count() | ||
|
|
||
| # TODO: RUSTPYTHON, parent method is marked but this version passes |
There was a problem hiding this comment.
The test_repr_deep method in list_tests.CommonTest is no longer marked, so this comment is currently outdated.
2 participants
No description provided.