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

Rationalize test fixtures #42

Merged
merged 28 commits into from
Jan 16, 2020
Merged

Rationalize test fixtures #42

merged 28 commits into from
Jan 16, 2020

Conversation

rod-glover
Copy link
Contributor

@rod-glover rod-glover commented Jan 15, 2020

Resolves #41

The main motivation for this PR is, as noted in #41, is to pave the way for a reasonable implementation of #32 (variable schema name), which is almost complete but foundering on test insanity.

There are now substantially fewer and simpler test fixtures, and they are much more consistent with each other.

Rationalizing the test fixtures led naturally to a hierarchy of test subdirectories, with test-specific fixtures isolated in deeper conftest.py files. This makes the relationship of the fixtures to each other and to test code much easier to understand, and the tests are now better organized and more navigable too.

Also removed a pile of cruft and orphaned code.

Notes to reviewers:

  1. The tests proper have hardly changed at all. You can safely ignore all files of the form test_*.py, and assume (rightly) that they have simply been moved to a new location and wired up to the new, rationalized fixtures.
  2. All of the important changes are in the various conftest.py files.

@rod-glover
Copy link
Contributor Author

rod-glover commented Jan 15, 2020

There is still some DRYing up of code that could be done:

  • creation and removal of a set of views in a fixture is copy-pasted in several places
  • addition of test database objects from pycds/data/crmp_subset_data.sqlis repeated

@rod-glover
Copy link
Contributor Author

OK, that's it. As squeaky-clean as it's going to get.

@rod-glover
Copy link
Contributor Author

This proved out by making the implementation of #32 very straightforward (in the testing realm). Since I need to move forward with this sequence of PRs, I am going to merge this one without review.

@rod-glover rod-glover merged commit f6af254 into master Jan 16, 2020
@rod-glover rod-glover deleted the i41-clean-up-tests branch January 16, 2020 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rationalize test fixtures
1 participant