Skip to content
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

Refactor the tests to be clearer and better differentiated between integration and unit #164

Closed
surchs opened this issue Jun 15, 2023 · 3 comments · Fixed by #385
Closed
Labels
flag:discuss Flag issue that needs to be discussed before it can be implemented. _flag:stale [BOT ONLY] Flag issue that hasn't been updated in a while and needs to be triaged again maint:coverage Test coverage improvements that were not included in feature prioritization maint:refactor Simplifying or restructuring existing code or documentation. type:maintenance Upkeeping efforts & catch-up corrective improvements that are not Features nor Bugs

Comments

@surchs
Copy link
Contributor

surchs commented Jun 15, 2023

We now have a lot of example test files with very minimal differences between them. That is because we are running most of our tests as integration tests (e.g. run through the entire app and looking at it from the outside). That makes sense to make sure that certain input files lead to certain behaviour of the entire app. I don't think this makes as much sense for asserting that very specific error messages are raised or warnings given. For this, we could just use a unit test of the function that does the raising or warning.

In return, we might be able to condense our examples to a couple of "valid" ones with some allowed variability, a couple of ones that should warn, and a couple that should fail outright. it will also make understanding the tests easier and be clearer to reason about if and when these tests fail.

Other potential options:

  • order the tests in some semantically meaningful way / split them into more test modules so we know where to look for and where to add similar tests
  • set up pytest markers for e.g., integration vs. unit tests following https://docs.pytest.org/en/7.1.x/how-to/mark.html
@surchs surchs added flag:discuss Flag issue that needs to be discussed before it can be implemented. type:maintenance Upkeeping efforts & catch-up corrective improvements that are not Features nor Bugs maint:coverage Test coverage improvements that were not included in feature prioritization maint:refactor Simplifying or restructuring existing code or documentation. labels Jun 15, 2023
@github-actions
Copy link

We want to keep our issues up to date and active. This issue hasn't seen any activity in the last 30 days.
We have applied the stale-issue label to indicate that this issue should be reviewed again and then either prioritized or closed.

@github-actions github-actions bot added the _flag:stale [BOT ONLY] Flag issue that hasn't been updated in a while and needs to be triaged again label Jul 16, 2023
@github-actions
Copy link

We want to keep our issues up to date and active. This issue hasn't seen any activity in the last 30 days.
We have applied the stale-issue label to indicate that this issue should be reviewed again and then either prioritized or closed.

@alyssadai
Copy link
Contributor

Since there's already some natural differentiation between the test_cli_*.py tests (ones that use the CLIRunner) and test_*util.py tests (ones that don't use the CLIRunner), I'll attempt to address this by refactoring the tests into integration and unit subdirectories for more top-level organization. This hopefully will also give us a bit more flexibility than relying on string matching pytest markers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flag:discuss Flag issue that needs to be discussed before it can be implemented. _flag:stale [BOT ONLY] Flag issue that hasn't been updated in a while and needs to be triaged again maint:coverage Test coverage improvements that were not included in feature prioritization maint:refactor Simplifying or restructuring existing code or documentation. type:maintenance Upkeeping efforts & catch-up corrective improvements that are not Features nor Bugs
Projects
Status: Review - Done
Development

Successfully merging a pull request may close this issue.

2 participants