Feature/setup basic e2e testing framework#105
Open
max-uni-hd wants to merge 26 commits into
Open
Conversation
b3da957 to
48ab145
Compare
When running a certain migration (tom_targets/migrations/0025_auto_20250206_2017.py) on an empty database, the migration crashed. A fix was introduced (TOMToolkit/tom_base@467174c) and shipped with version 2.28.1. This commit updates the tomtoolkit to a version that includes this fix.
I split up the docker compose file for the different use-cases. The local one for example should allow faster development cycles because it uses django's dev server that has auto reload on changes. Disclaimer: I am not a Docker expert by any means, but I tried to restructure the Dockerfile to some extent, because I think that might have some positive impact on caching the individual layers during builds.
This is a very basic version that still needs some work but should be ok as a starting point.
The django testrunner resets the database for unit and integration tests. So far, the container is only added when running compose.local.yaml.
- changed to smaller container for runner container - split entrypoints - add seeding endpoint only for tests to restore DB for e2e tests
Mostly removed duplication
There was a <th> in a body row.
Also some minor whitespace changes.
We plan on mounting the database fixture on the staging system so we don't have to commit the fixture in case it contains proprietary data. This commit adds the mounts and path changes needed for this to work, hopefully.
No longer needed.
The e2e tests sometimes fail because the first 2 targets switch positions. I am not sure why that is, but it might have mostly to do with .filter returning elements arbitrarily. I formatted the code a bit so the lines aren't that long anymore. Then, I mostly added "-id" in the sorting as a final "tie breaker" (that was the intetion at least). And the results are picked from the query set using the sorted prio_ids. The tests now seem to always pass.
GalacticTarget was updated and now has new fields that also need to be filled in the e2e tests. This was missing and caused test failures.
QuantileTransformer was imported in apps.py but not used.
This reverts commit 21ab8e5.
After talking to Markus, the changes in views.py were reverted and instead the tests were tuned to be more relaxed on the elements appearing on the home page.
48ab145 to
9da6d01
Compare
mpgh
reviewed
May 12, 2026
Contributor
There was a problem hiding this comment.
In the long run, the lightcurves should always be available. If it needs to be, empty. The radar plot, however, is tied to the microlensing science case and we expect it will look different for the second use case.
mpgh
reviewed
May 12, 2026
mpgh
reviewed
May 12, 2026
mpgh
reviewed
May 12, 2026
mpgh
reviewed
May 12, 2026
Contributor
mpgh
left a comment
There was a problem hiding this comment.
I have checked the nominal structure. Details w.r.t. prod are left for @astroadmin to comment on. As far as the plots are concerned, some are required to appear, some are optional. This is not reflected in the code base, yet. For the time being, this looks reasonable.
This file contains hidden or 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
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 is unfortunately a rather big PR. Some information up front:
The purpose of this PR was to add a basic E2E testing setup and basic E2E tests to make it easier to spot errors when changes are made to the code base.
I also added some sample unit and integration tests.
I have tried to add everything you need to review in the README.md, starting at line 88.
But as a summary:
Important
For the E2E tests to work, you have to put the file
db_out_full.jsonin the folder specified in the env variableDJANGO_TEST_DATA_DIR.As this file might at some point contain proprietary data, it is not committed
for now. This might change in the future. To get this file, reach out to me (or if you have access to the server, it is under
/opm-data/test_data).If you want to run the e2e tests, use the following command:
Once the container is done, you should (hopefully) see something like this:
(Don't mind the warnings: in my case they are "just" deprecation notices.)
You can run them again locally (given you installed all the dependencies) via
pytest custom_code/tests/e2e -rsor viato start the container again.
Not "too production relevant" changes
The impact on the regular app should be small or non existent, because the only changes that were made here were:
These should be all the files with such changes:
Most of the other changes are under custom_code/tests and should have no impact.
Changes with impact on app
I have refactored the way we use
docker composequite a bit and that will have an impact on the startup scripts on the server.I also made some changes to the Dockerfile, but nothing major, I think. It ran on my machine™. I hope it also runs on the server. Once the staging environment is done, I will test it thoroughly.