Test on Python 3.7 and Python 3.8 #84
Conversation
This file was using the `unittest` framework from the standard library, but we invoke it through pytest. Remove the class inheriting from unittest.TestCase and promote the methods to module-level functions, which pytest's introspection will still find. The lexer member variable is replaced with a pytest fixture. This removes a level of indirection (and indentation) and makes it possible to use more advanced pytest features like parametrization.
Use pytest's parametrization decorator instead of explicitly looping with a helper. This gives finer-grained test feedback.
This set of tests were written to use the `unittest` framework from the standard library but we were invoking them through pytest. Remove the class and promote its methods to module-level functions. The setUp method wasn't actually used, and self was only used to lookup staticmethods, so there are otherwise few changes.
The open_file function is never called. The write_file function is called once, but nothing is done with the result. If access to test files is eventually needed, for example to implement test_text_code_and_description, they're already available through the the test.fixtures module.
Use a variable to compare input with serialization instead of repeating the same text. This is less error-prone on update.
These are no longer necessary for the current version of the code.
Since we call the parser and serializer with the default parameters, there's not much value to wrapping the actual API calls this way. The prefix/imperative style may be slightly easier to read than the suffix/functional style, but I prefer making explicit what functions are being called in the test.
None of the tests calling `compare_tokens` makes use of the lexer fixture after passing it in. We can therefore avoid needing to pass it through from every test by just creating a local lexer inside that function.
Add a job testing on Python 3.7, which is the current stable release. This required porting away from mixed pytest and unittest for the particular versions of those libraries available to the job. Python 3.8 isn't in stable release yet, but it's good to get advance warning on portability issues.
|
@ageorgou ready for review. |
|
@rillian I'm afraid I won't have time to review until next week, but I'll take a look as soon as I can. Thanks! |
|
No worries. Thanks for the update! |
| @@ -113,23 +113,21 @@ def test_text_code_and_description(): | |||
| """ | |||
| Check if serializing works for the code/description case - first line | |||
| of ATF texts. | |||
| Note the parser always returns an AtfFile object, even when it's not | |||
| ATF-compliant. | |||
There was a problem hiding this comment.
Is this no longer true (from previous updates), or did you just not consider it relevant? I'm guessing that it was here to make it clear that the test is meaningful, as the serializer always works on an AtfFile object rather than a plain string (although with the more transparent method calls, maybe this doesn't matter).
There was a problem hiding this comment.
I guessed similarly. I removed it because don't think it's correct for the current code. In the case of the project line test:
>>> from pyoracc.atf.common.atffile import AtfFile
>>> AtfFile("#project: cams/gkab\n")
[...]
SyntaxError: PyOracc could not parse token PROJECT at line 1 at offset 1 with value 'project'.i.e. it fails without the preceding code and description line. I suppose it's partially correct in that if the required lines occur in order it will return an object with the missing values filled in with defaults, as happens with the code test, but either way I didn't find it helpful enough to include in the docstring.
These tests all look pretty easy to fix up, but I thought that should be a different PR.
There was a problem hiding this comment.
Makes sense, thanks for catching that then!
Remove unittest dependency, addressing #82, which unblocks adding travis jobs for current and upcoming python releases.