Skip to content

ci: Run the test suite for PRs and pushes#3658

Merged
NGPixel merged 34 commits into
ietf-tools:feat/bs5from
larseggert:ci-run-tests
Mar 19, 2022
Merged

ci: Run the test suite for PRs and pushes#3658
NGPixel merged 34 commits into
ietf-tools:feat/bs5from
larseggert:ci-run-tests

Conversation

@larseggert

Copy link
Copy Markdown
Collaborator

No description provided.

@larseggert

Copy link
Copy Markdown
Collaborator Author

Test is failing because of the <a> issue in base.html and I don't have permissions to re-run it. I guess this failure demonstrates that it works though :-)

Comment thread .github/workflows/ci-run-tests.yml Outdated
@larseggert
larseggert requested a review from NGPixel March 16, 2022 16:16
@NGPixel

NGPixel commented Mar 16, 2022

Copy link
Copy Markdown
Member

The image build alone takes almost 10 minutes to complete... It might be best to precompile a test image instead.

@larseggert

Copy link
Copy Markdown
Collaborator Author

Yes, at least the apt stuff could be canned.

@NGPixel

NGPixel commented Mar 17, 2022

Copy link
Copy Markdown
Member

@larseggert I made some significant changes to the workflow so that it uses the "datatracker-test-base" image (which is automatically built when either package.json or requirements.txt get modified). This reduces the setup time from over 10 mins, down to just 2 mins.

The image is essentially the same as the dev image, but uses python:3.6-bullseye as base instead and it removes some dev specific installs / configs that are not relevant for testing.

The workflow now run directly from inside the test container instead of launching a docker-compose. I disabled the MariaDB container as we use sqlite during tests, but we can re-enable it in the future if needed.

Right now it fails at the run tests step as it wants to apply migrations first. However, adding manage.py migrate just before fails as it expects some tables to be present already...

@larseggert

Copy link
Copy Markdown
Collaborator Author

Right now it fails at the run tests step as it wants to apply migrations first. However, adding manage.py migrate just before fails as it expects some tables to be present already...

Do you have a resolution to that?

@rjsparks

Copy link
Copy Markdown
Member

The test runner applies migrations automatically - if it's running into trouble saying it can't touch a table, someone may have added some code that tries to use tables before django is actually initialized. Can you give me a link to the failure?

Fwiw, ietf/manage.py migrate by itself will migrate the database configured in settings/settings_local. The tests are using the database configured in settings__sqlitetest. If you're not invoking the tests with --settings=settings_sqlitetest, they will fail. And do not try to run migrate with --settings=settings_sqlitetest - It may or may not succeed, but after you read about what the test runner does, you'll see that's non-sensical.

@NGPixel

NGPixel commented Mar 19, 2022

Copy link
Copy Markdown
Member

Finally got the tests to run correctly. The culprit was a missing docker-cli on the base test image, which is somehow needed to talk to the DB.

@NGPixel
NGPixel merged commit 05c6439 into ietf-tools:feat/bs5 Mar 19, 2022
@larseggert
larseggert deleted the ci-run-tests branch March 23, 2022 10:16
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jun 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants