Shuffle tests

[kiblik]

Test robustness of unittests. They should pass in any order.

[dryrunsecurity]

Hi there 👋, @DryRunSecurity here, below is a summary of our analysis and findings.

DryRun Security	Status	Findings
Server-Side Request Forgery Analyzer	✅	0 findings
IDOR Analyzer	✅	0 findings
Authn/Authz Analyzer	✅	0 findings
Configured Codepaths Analyzer	✅	0 findings
Sensitive Files Analyzer	✅	0 findings
SQL Injection Analyzer	✅	0 findings
Secrets Analyzer	✅	0 findings

Note

🟢 Risk threshold not exceeded.

Change Summary (click to expand)

The following is a summary of changes in this pull request made by me, your security buddy 🤖. Note that this summary is auto-generated and not meant to be a definitive list of security issues but rather a helpful summary from a security perspective.

Summary:

The provided code changes are focused on improving the unit testing setup and execution for a Django-based application. The key changes include:

1. Randomizing the order of unit test execution using the --shuffle option, which can help identify any potential dependencies or order-specific issues in the test suite.
2. Ensuring that the correct database and Celery broker configurations are used for the test environment, preventing any unintended access to production resources during testing.
3. Validating the REST API schema annotations to ensure that the API documentation and client-side code generation are accurate and secure.
4. Enforcing the creation of database migrations for model changes, maintaining the integrity and consistency of the application's data model.

These changes are not directly related to security concerns, but they demonstrate a comprehensive approach to unit testing that can help identify and prevent potential security vulnerabilities early in the development lifecycle. Maintaining a robust and well-structured unit test suite is a good practice from an application security perspective, as it can catch issues before they are introduced into the production environment.

Files Changed:

1. docker/entrypoint-unit-tests-devDocker.sh:

   • Added the --shuffle option to the python3 manage.py test unittests command, which randomizes the order of unit test execution.
   • Implemented steps to set up the necessary environment for running the unit tests, such as loading secret environment variables, connecting to the database, running database migrations, and checking the Django application for any issues.

2. docker/entrypoint-unit-tests.sh:

   • Ensured that database migrations are created and applied before running the unit tests.
   • Unset the database URL and Celery broker URL environment variables to force the use of specific configurations for testing.
   • Checked if the REST API schema annotations are correct by running the python3 manage.py spectacular command.
   • Checked if there are any pending model changes without corresponding database migrations.
   • Ran the unit tests using the python3 manage.py test command with the --keepdb, --no-input, and --shuffle options.

These changes demonstrate a comprehensive approach to unit testing that can help improve the overall security and reliability of the application.

Powered by DryRun Security

[Maffooch]

I ran into some pickling errors with parallel turned out. I am not really sure what we would need to do to get that working. If shuffle works here, I think that would still make for a pretty big win 😀

[kiblik]

I ran into some pickling errors with parallel turned out. I am not really sure what we would need to do to get that working. If shuffle works here, I think that would still make for a pretty big win 😀

Yes, I was pleasantly surprised by it as well.
I do not see some easy fix for --parallel so I will keep only --shuffle here and I will try to fix --parallel in separate PR.

[mtesauro]

Approved

[Maffooch]

@kiblik I think tests were passing here by coincidence like the last time we tried something like this.. Very frustrating... We're going to revert for now, and then fix more widely in the future. Once those fixes are applied, we can enable shuffle again

I found something out about this one

I ran into some pickling errors with parallel turned out. I am not really sure what we would need to do to get that working.

The error raises from an exception/stacktrace during the test that is not able to be pickled. I think exceptions are intended to be handled, and then the test will fail via the assertions.

[kiblik]

@kiblik I think tests were passing here by coincidence like the last time we tried something like this.. Very frustrating... We're going to revert for now, and then fix more widely in the future. Once those fixes are applied, we can enable shuffle again

I found something out about this one

I ran into some pickling errors with parallel turned out. I am not really sure what we would need to do to get that working.

The error raises from an exception/stacktrace during the test that is not able to be pickled. I think exceptions are intended to be handled, and then the test will fail via the assertions.

I suppose the problem with pickling is mainly in --parallel (#10408). But I agree that --shuffle is not behaving "well". I planned to offer to exclude unittests.test_rest_framework and run it separately without --shuffle. But full revert is probably better idea for now.