Overhaul test suite with fixtures #3248
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This creates a PostgreSQL fixture that can be depended on by tests that require a running PostgreSQL server with the NAV database to be present. The fixture will work with externally set up PostgreSQL servers, like in the case of GitHub Actions, while it can also provision a PostgreSQL server with the NAV schema using Docker if run locally.
This ensures any tests that rely on the various pre-existing db-related fixtures will have a running PostgreSQL server available.
This ensures the postgres fixture is started for all tests that inherit from DjangoTransactionTestCase
Change TestCase implementations that should really be transactional test cases (which in turn ensures they have PostgreSQL available through the postgres fixture, and that transactions are properly rolled back after the tests have run)
Declares a dependency on the postgresql fixture to ensure the NAV database is actually available for these tests
This ensures that any test that depends on a logged in web client also automatically depends on having the NAV database running.
This ensure the snmpsim fixture can run both on Linux and MacOS, by replacing Linux specific parts (like looking at /proc/net/udp). This slightly changes the logic of verifying whether the snmpsimd process is up and running before yielding the fixture value: Instead of waiting for something to appear to listen to port 1024, this verifies the presence of the SNMP agent by performing an actual SNMP query.
These imports made the entire test module depend on the PostgreSQL database actually running. Moving the imports to be local to the tests (that depend directly or indirectly) on the postgresql fixture ensures that the imports only take place after PostgreSQL is actually running.
The admin_navlet fixture is dynamic/parametrized, which is why its implementation is a bit weird. Unfortunately, the tests that depend on it currently fail under the new regime of the postgresql fixture. Adding this note so we know where to go back and fix it.
While pytest can accomplish a lot of exciting things, it cannot use fixtures as input to test parametrization. While can make a test depend on a fixture for getting a postgresql instance, the test discovery phase that generates the tests cannot. I.e. we cannot get our test parameters from a database unless the database was already up and running when the test discovery phase started. Sad, but true,. This changes the navlet test to iterate over the available admin navlets itself.
These tests would only work incidentally; i.e. if they ran after another test that depended on the postgresql fixture
The postgresql fixture is fine to depend on if you just need a database to be present. But if you need to write data during the test, you should probably use the db fixture.
For now, at least. If the Open-LDAP libraries aren't available locally, the test suite will fail because python-ldap will not build.
The NAV_CONFIG_DIR environment variable can be used to insert a custom directory as the first entry in the list of directories NAV will search for its config files.
Both the SQL statement and the command line were incorrect.
The postgresql fixture needs to pass on the admin user's name and password as fetched from the fixtures, in order for the test database to be properly populated.
This ensures static files are collected only by tests that require it. It also ensures that the static files are collected into a temporary directory and that the WhiteNoise web server is directed to base its document root on this path (so we don't try to write to some system level directory we don't have write access to)
This ensures stylesheets are built only on-demand
Instead of running it as part of tox, this ensures it runs as an explicit test in the integration test suite.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3248 +/- ##
===========================================
- Coverage 60.30% 38.19% -22.11%
===========================================
Files 605 605
Lines 43685 43691 +6
Branches 48 48
===========================================
- Hits 26343 16688 -9655
- Misses 17330 26991 +9661
Partials 12 12 ☔ View full report in Codecov by Sentry. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR has an overarching goal of making the test suite easier to run locally, rather then requiring devs to run all tests inside a very specifically defined Docker container (which is NOT how the test suite is run on GitHub actions).
Goals:
conftest.py, such as gunicorn, PostgreSQL, Xvfb, while expecting that these are available in the local environment, these services should be defined as test fixtures. Any test that depends on PostgreSQL should declare a dependency to this fixture, in order for it to be able to use PostgreSQL.Some of the changes herein would probably stand to be factored out into separate PRs, as there are many fixes to the existing test suite that are not dependent on the fixture changes made.
Many of the original changes of this PR have already been factored out into separate PRs that have been merged, this PR represents what remains to be sorted out, and is kept as a draft for the time being.
(This PR replaces #2675, which was automatically closed by GitHub due to branch changes)