-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Port unittest
classes to pytest
.
#82
Comments
Thanks for raising this. I'm not sure what led to mixing both frameworks, but replacing I'm not sure this is the only reason for the failure with specific Python/pytest versions, but at least it explains why |
I see you assigned this to yourself. If it's helpful, I did some work on it. I didn't parametrize compare tokens. It would remove passing through the fixture (maybe there's a more clever way to do that?) but we'd lose the specificity of the function names, and the comparison entries are hard enough to read one line at a time. Maybe you have better ideas? |
By all means, feel free to contribute! I think leaving I think you can avoid passing the lexer fixture explicitly in each test function with the autouse option. I also wonder whether we need one lexer per test, or whether it can be a module-scoped fixture (or class-scoped, if keeping all the tests in a class as before). |
Ok, great. It does seem like we don't really need separate parsers for each test, but it's better hygene to do so in case they develop more internal state at some point. It looks like none of the tests calling FWIW, porting |
After #84, I think this only leaves The simplest way I see is to turn the lexer into a pytest-style fixture. The tests could also move at module-level rather than a class, but that would entail more changes and isn't necessary. diff --git a/pyoracc/test/atf/test_atfparser.py b/pyoracc/test/atf/test_atfparser.py
@@ -19,7 +19,6 @@ along with PyORACC. If not, see <http://www.gnu.org/licenses/>.
'''
-from unittest import TestCase
import pytest
from pyoracc.atf.common.atflex import AtfLexer
@@ -39,9 +38,10 @@ from ...model.text import Text
from ...model.translation import Translation
-class TestParser(TestCase):
+class TestParser:
"""A class that contains all tests of the ATFParser"""
- def setUp(self):
+ @pytest.fixture(autouse=True)
+ def lexer(self):
self.lexer = AtfLexer().lexer The test functions only use the lexer in the Any thoughts, @rillian? |
That's certainly the minimal change. You could even leave the method name the same! If you don't have immediate plans for tests which need to access the lexer or parser state before or after calling In my experience, readability now is more important that ease of addition later, so this is the stronger argument. The github review interface is pretty good about marking indent-only changes, so making If you do remove the class wrapper, I'd commit your minimal change first so there's an easier point to revert to if you need shared state later. |
Closing this after #85, but feel free to reopen if you think there is something else left. |
LGTM :) |
Many of the unit tests are written the Python standard library's
unittest
framework. For automated testing the externalpytest
package is used. Normally that's fine, but in preparing #81 I ran into the issue that pytest doesn't support parametrization of unittest methods.At least for the file I was working on,
test_aftlex.py
, it looked like theunittest
functionality could be replaced by a pytestfixture
. Moving the tests up to module level would reduce indentation and allow cleaner reporting of parameterized tests.Less worthy motivation, but possibly also expedient, is that travis builds fail on Python 3.7.1 in the unittest loader. I don't see this locally with Python 3.7.3, so issue may be addressed by more up-to-date Python packages, but dropping the dependency might work around the bug.
The text was updated successfully, but these errors were encountered: