Skip to content

Feature/setup basic e2e testing framework#105

Open
max-uni-hd wants to merge 26 commits into
devfrom
feature/setup-basic-e2e-testing-framework
Open

Feature/setup basic e2e testing framework#105
max-uni-hd wants to merge 26 commits into
devfrom
feature/setup-basic-e2e-testing-framework

Conversation

@max-uni-hd
Copy link
Copy Markdown
Collaborator

@max-uni-hd max-uni-hd commented May 6, 2026

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.json in the folder specified in the env variable DJANGO_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:

docker compose -f compose.base.yaml -f compose.prod.yaml -f compose.e2e.yaml up -d
# this will start up all the containers and run the e2e container, which runs the tests as well
# so, run the next command also to see what this container is doing
docker compose -f compose.base.yaml -f compose.prod.yaml -f compose.e2e.yaml logs -f e2e

Once the container is done, you should (hopefully) see something like this:

e2e-1 | ============ 41 passed, 2 skipped, 48 warnings in 102.09s (0:01:42) ============

(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 -rs or via

docker compose -f compose.base.yaml -f compose.prod.yaml -f compose.e2e.yaml up \
--abort-on-container-exit --exit-code-from e2e

to 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:

  • add data-testid attributes for easier selection in templates
  • some whitespace changes
  • fix minimal html "errors" that were causing problems in tests
  • add 2 endpoints for testing

These should be all the files with such changes:

  • custom_code/templates/custom_code/prob_list.html
  • templates/tom_common/index.html
  • templates/tom_registration/partials/pending_users.html
  • templates/tom_targets/partials/aladin_finderchart.html
  • templates/tom_targets/partials/target_data.html
  • templates/tom_targets/target_detail.html
  • templates/tom_targets/target_grouping.html
  • templates/tom_targets/target_list.html
  • templates/tom_targets/target_list.html
  • custom_code/views.py
  • galactic_science_opm/urls.py

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 compose quite 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.

@max-uni-hd max-uni-hd marked this pull request as ready for review May 6, 2026 11:14
@max-uni-hd max-uni-hd requested review from astroadmin and mpgh May 6, 2026 11:14
@max-uni-hd max-uni-hd force-pushed the feature/setup-basic-e2e-testing-framework branch from b3da957 to 48ab145 Compare May 12, 2026 07:24
Maximilian Kistner and others added 26 commits May 12, 2026 10:46
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.
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.
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.
@max-uni-hd max-uni-hd force-pushed the feature/setup-basic-e2e-testing-framework branch from 48ab145 to 9da6d01 Compare May 12, 2026 08:59
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread custom_code/views.py
Comment thread custom_code/tests/e2e/test_target_page.py
Comment thread custom_code/tests/e2e/test_target_page.py
Copy link
Copy Markdown
Contributor

@mpgh mpgh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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.

2 participants