Conversation
- Remove unused imports. - Replace wildcard imports with explicit imports. - Normalize ordering: stdlib, third-party, local. - Ensure no docstrings on tests; remove comments that restate code.
- Moved helper methods out of classes in tests/test_cli.py, tests/ test_commcare_hq_client.py, tests/test_commcare_minilinq.py, tests/ test_checkpointmanager.py, tests/test_excel_query.py, tests/ test_minilinq.py, tests/test_misc.py, tests/test_writers.py. - Flattened simple single-purpose classes to module-level tests in tests/ test_map_format.py, tests/test_paginator.py, tests/ test_repeatable_iterator.py, and the digest tests in tests/test_misc.py. - Standardized helper usage and removed class-local utility methods in SQL writer tests (now _type_convert, _check_excel2007_output, _test_types, _get_column_lengths are module-level).
- Flattened repetitive value-or-root tests into a single parametrized case and fixed a typo’d test name in tests/test_minilinq.py. - Consolidated CommCare HQ client iteration helper usage and simplified retry header tests into a parametric check in tests/test_commcare_hq_client.py. - Standardized writer test names and moved Excel/SQL helper logic to top-level functions in tests/test_writers.py. - Updated names where needed for clarity.
kaapstorm
force-pushed
the
nh/refactor_tests
branch
from
61b57d5 to
0d74cb2
Compare
| """ | ||
| Test that we continue with the same pagination mode as was | ||
| already in use | ||
| """ |
There was a problem hiding this comment.
I'm curious about the motivation for removing docstrings from tests? Does it affect the output of pytest? If yes, maybe it would make sense to adjust pytest's output formatter rather than removing docstrings?
It seems like we're losing something by deleting them.
There was a problem hiding this comment.
the motivation for removing docstrings from tests
If I remember correctly, there was a conversation about this regarding commcare-hq a few years ago -- that we should rather use well-named test methods than docstrings. I used to use docstrings on tests, but since then I adopted this as our coding style.
I agree that sometimes we are losing something.
I'm keen to hear your thoughts on this:
- Sometimes use docstrings on tests?
- Never use docstrings on tests, but do use comments in place of docstrings?
- Something else
There was a problem hiding this comment.
I don't usually put docstrings on tests, and instead prefer to assign meaningful function names. Back in the nosetests days docstrings were annoying because they were preferred over the test fully qualified name in nose output, which was not as easy to copy/paste to re-run just that test. However, I'm pretty sure we had a nose plugin that always printed fully qualified test names, so then it was not as much of an issue.
I'm not sure how they are handled by pytest. Do they make the output more or less readable? If they do make it less readable, can that be changed through configuration or extension?
I certainly don't have a problem with adding a comment to clarify meaning of test code, and therefore don't have a problem with docstrings if it doesn't impact the convenience of running tests.
There was a problem hiding this comment.
pytest output is not affected by docstrings. So I've put them back, and for parametrized tests I've put them back as comments. I've also removed the guidance in CONTRIBUTING.md not to use docstrings in tests. a1cf21d
Update guidance in CONTRIBUTING.md
2 participants
A sweeping 😉 cleanup of tests.
No tests were added or removed. These changes are stylistic and structural. They aim to get closer to the coding style given in
CONTRIBUTING.md.Note
Base branch is not
master🐡