gh-109485: Further improve test_future_stmt tests
#109486
Conversation
sobolevn
requested review from
brettcannon,
ericsnowcurrently,
ncoghlan and
warsaw
as code owners
vstinner
left a comment
vstinner
left a comment
There was a problem hiding this comment.
Would it be possible to check for the error message in assertSyntaxError?
| self.assertSyntaxError(code, 3) | ||
|
|
||
| def test_badfuture10(self): | ||
| def test_bad_future_as_module(self): |
There was a problem hiding this comment.
Test name is not clear. The expected error is:
SyntaxError: from __future__ imports must occur at the beginning of the file
|
With error messages it should be much better! |
| lineno, | ||
| message=TOP_LEVEL_MSG, offset=1, | ||
| parametrize_docstring=True): | ||
| code = dedent(code) |
There was a problem hiding this comment.
It seems like code first line is always empty. I suggest using:
| code = dedent(code) | |
| code = dedent(code.lstrip()) |
And adapt lineno -= 2 and other references to lineno below.
For example, for me, test_unknown_future_flag() code has only 3 lines and expect an error on the 4th line, it's strange.
vstinner
left a comment
vstinner
left a comment
There was a problem hiding this comment.
LGTM. Thanks! It's way way more readable now! The test methods have better name, it's easier to understand the purpose of the test, and you added many checks!
| code = dedent(code.lstrip('\n')) | ||
| for trim_docstring in ([False, True] if parametrize_docstring else [False]): | ||
| with self.subTest(code=code, trim_docstring=trim_docstring): | ||
| if trim_docstring: |
There was a problem hiding this comment.
Last question: instead of copy/paste '''Docstring''' in each test. Would it make sense to add it here instead? I'm waiting for your opinion to decide if we merge the change like that, or you prefer to remove docstrings in tests and add it here instead.
|
Thanks a lot @sobolevn, it's way better! I edited the final message. |
2 participants
I've also enabled docstring parametrization, because tests must works the same way with and without docstrings most of the time.
test_future_stmt#109485