Skip to content

Enhancement/refactor tests#4221

Open
jonbulz wants to merge 7 commits intodevelopfrom
enhancement/refactor-tests
Open

Enhancement/refactor tests#4221
jonbulz wants to merge 7 commits intodevelopfrom
enhancement/refactor-tests

Conversation

@jonbulz
Copy link
Copy Markdown
Contributor

@jonbulz jonbulz commented Mar 26, 2026

Short description

Refactoring of our test suite to improve test reliability and execution time. Adds a comprehensive guide on how to use the introduced arguments and how to add new tests.

Proposed changes

Unify CI setup and local runs

  • introduce test_settings module to export the same settings to circleci_settings and test.sh, eliminating duplicated dummy API keys and feature flags
  • explicitly export DJANGO_SETTINGS_MODULE in test.sh after require_database, which was overriding with the base settings

Fix flaky tests and eliminate ordering dependencies

  • remove the complex relative after_tests tuples and inter-test dependencies. Retain a simple @pytest.mark.order("last") on all transactional tests (needed because the --circleci-parallelize plugin reorders tests by module after pytest-django's collection).
  • testmon + xdist race condition — --testmon-noselect conflicted with parallel workers, causing sporadic User.DoesNotExist errors during fixture setup. Fixed by disabling testmon in parallel mode (-p no:testmon).
  • POI category test bug — test_edit_poi_category_was_successful was asserting the wrong category name in the success message, and the session-scoped client's message cookie would overflow in the full parallel suite. Fixed by checking database state directly instead of HTML messages.
  • re-enable tow HIX tests that were skipped by converting the session-scoped dummy_region fixture to function-scope
  • remove serialized_rollback=True from django_db markers. it is no longer needed because ordering dependencies are eliminated, and removing this argument significantly speeds up transactional tests
  • mark test_rule_out_false_positive as xfail(strict=False) because it is non-deterministic by nature

Improve test performance

  • introduce unit and slow test markers
  • add QUICK_ROLES for faster local iteration
  • auto-mark all status-code view tests as slow via directory-level conftest.py

Improve test infrastructure

  • add tests/factories.py for cleaner test setups
  • extract role constants into tests/constants.py (importable without triggering fixture registration side effects)
  • allow to update snapshots in a single test run with update_snapshots argument

Documentation

  • add comprehensive testing guide in docs/src/testing.rst

Side effects

  • Introduces test coverage threshold. If we fail to add tests for new features, the pipeline will fail in the future

Faithfulness to issue description and design

There are no intended deviations from the issue and design.

How to test

Run the test suite on your local machine. See that tests are now deterministic. Try the different options and see that they reduce test execution time:

  • ./tools/test.sh -m unit — unit tests only (seconds)
  • ./tools/test.sh -m "not slow" — skip slow parametrized view tests
  • QUICK_ROLES=1 ./tools/test.sh — test only 4 representative roles

Resolved issues

Fixes: #2711 (sort of)


Pull Request Review Guidelines

@jonbulz
Copy link
Copy Markdown
Contributor Author

jonbulz commented Mar 26, 2026

@PeterNerlich interested in your opinion here :)

@MizukiTemma
Copy link
Copy Markdown
Member

All the tests fail due to django.db.utils.OperationalError: connection failed: connection to server at "127.0.0.1", port 5432 failed: Connect... whenever I try test.sh. I assume it is brought by this PR, as it works on the develop branch. Could you check once? I could not solve by mylself 🥺

@PeterNerlich
Copy link
Copy Markdown
Contributor

PeterNerlich commented Apr 6, 2026

All the tests fail due to django.db.utils.OperationalError: connection failed: connection to server at "127.0.0.1", port 5432 failed: Connect...

Same for me.

This is the difference in os.environ between develop and this branch
$ diff -u0 develop.env broken.env 
--- develop.env	2026-04-07 00:28:38.136035965 +0200
+++ broken.env	2026-04-07 00:25:33.852713670 +0200
@@ -5,3 +4,0 @@
- 'INTEGREAT_CMS_TEXTLAB_API_KEY': 'dummy',
- 'INTEGREAT_CMS_DEEPL_AUTH_KEY': 'dummy',
- 'INTEGREAT_CMS_GOOGLE_PROJECT_ID': 'dummy',
@@ -27,2 +24 @@
- 'INTEGREAT_CMS_BACKGROUND_TASKS_ENABLED': '0',
- 'DJANGO_SETTINGS_MODULE': 'integreat_cms.core.docker_settings',
+ 'DJANGO_SETTINGS_MODULE': 'integreat_cms.core.test_settings',
@@ -39 +34,0 @@
- 'INTEGREAT_CMS_SUMM_AI_API_KEY': 'dummy',
@@ -44 +38,0 @@
- 'INTEGREAT_CMS_LINKCHECK_DISABLE_LISTENERS': '1',
@@ -46 +39,0 @@
- 'INTEGREAT_CMS_GOOGLE_CREDENTIALS': 'dummy.json',

I guess that at least currently, this PR implies that one needs to use a dedicated database installation instead of a docker container. Where until now require_database() in tools/_functions.sh decided which settings module to use depending on what it detected on the system, tools/tests.sh now overwrites that environment variable to use the test settings.
We probably want to change that somehow.

@jonbulz
Copy link
Copy Markdown
Contributor Author

jonbulz commented Apr 7, 2026

No, I just assume that your container is listening on a different port. Because the settings have been unified, you now have to manually set the port as environment variable in that case, probably export INTEGREAT_CMS_DB_PORT=5433

@MizukiTemma
Copy link
Copy Markdown
Member

Thank you for reply, now I can run the tests with export INTEGREAT_CMS_DB_PORT=5433 :)

Were they deterministic in your local environment? After ./tools/test.sh or ./tools/test.sh -m "not slow", some tests always fail but others fail every n-time (for exmaple test_repair_tree)

@jonbulz
Copy link
Copy Markdown
Contributor Author

jonbulz commented Apr 9, 2026

Damn, it seemed deterministic to me 😅 Can you paste which tests are failing?

@MizukiTemma
Copy link
Copy Markdown
Member

These tests are always failing but pass when run individually

  • test_api_result
  • test_is_not_deletable_if_used_as_within_translation_of_upcoming_event
  • test_sitemap
  • test_auto_translate_easy_german
  • test_copy_pois_succeeds

These test are always failing and also fail when run individually

  • test_csv_export_feedback
  • test_summ_ai_error_handling

These test fail sometimes

  • test_fix_internal_links_dry_run
  • test_check_broken_tree_fields
  • test_fix_broken_tree_fields
  • test_repair_tree

@hannaseithe hannaseithe self-requested a review April 20, 2026 15:33
@hannaseithe
Copy link
Copy Markdown
Contributor

hannaseithe commented Apr 21, 2026

I ran the tests with export INTEGREAT_CMS_DB_PORT=5433
It says

5241 passed, 2 skipped, 1 xfailed, 1 xpassed, 1039 warnings in 395.98s (0:06:35)

but ALSO a super long stack trace output of

...
Message: 'repr() for object %s failed because of %s: %s (If you think this is no problem, please exclude this exception in the repr() method of the AbstractBaseModel.)'
Arguments: ('<PageTranslation (id: 2)>', 'RuntimeError', RuntimeError('Database access not allowed, use the "django_db" mark, or the "db" or "transactional_db" fixtures to enable it.'))

A second run gave me almost the same result, but this time I also have one failure and one error:

FAILED tests/core/management/commands/test_fix_internal_links.py::test_fix_internal_links_commit - AssertionError: Some old links should not exist after replacement
ERROR tests/core/management/commands/test_copy_pois.py::test_copy_pois_succeeds[--contacts --events --add-suffix] - django.db.utils.IntegrityError: Problem installing fixture '/home/hanna-seithe/integreat-cms/integreat_cms/cms/fixtures/test_data.json': Could not load linkcheck.Url(pk=20): duplicate key value violates unique constraint "linkcheck_url_url_key"
1 failed, 5239 passed, 2 skipped, 1 xfailed, 1 xpassed, 1011 warnings, 1 error in 360.11s (0:06:00)

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.

Make tests more independent and/or remove the role the test_data fixture plays

4 participants