Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor integration tests into matrix #3618

Merged
merged 6 commits into from
Sep 16, 2023
Merged

Refactor integration tests into matrix #3618

merged 6 commits into from
Sep 16, 2023

Conversation

tgaddair
Copy link
Collaborator

No description provided.

run: |
RUN_PRIVATE=$IS_NOT_FORK LUDWIG_TEST_SUITE_TIMEOUT_S=7200 pytest -v --timeout 300 --durations 100 -m "not slow and not combinatorial and not horovod and not llm and not integration_tests_a and not integration_tests_b and not integration_tests_c and not integration_tests_d" --junitxml pytest.xml tests/integration_tests
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@justinxzhao why is it that only test suite E had all these not integration_tests_a ... markers but the others don't?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, this is because tests for groups a/b/c/d, are explicitly marked with pytest.mark while the last bucket (in this case "e"), is a catch-all for all of the rest of unmarked tests. The thinking here was that folks adding a new file in integration_tests/ shouldn't have to think about assigning a marker.

@github-actions
Copy link

github-actions bot commented Sep 15, 2023

Unit Test Results

  4 files   -   2    4 suites   - 2   31m 41s ⏱️ - 10m 45s
31 tests ±  0  26 ✔️ ±  0    5 💤 ±0  0 ±0 
62 runs   - 20  52 ✔️  - 14  10 💤  - 6  0 ±0 

Results for commit 534ee42. ± Comparison against base commit 4806254.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@justinxzhao justinxzhao left a comment

Choose a reason for hiding this comment

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

Thanks for this refactor!

run: |
RUN_PRIVATE=$IS_NOT_FORK LUDWIG_TEST_SUITE_TIMEOUT_S=7200 pytest -v --timeout 300 --durations 100 -m "not slow and not combinatorial and not horovod and not llm and not integration_tests_a and not integration_tests_b and not integration_tests_c and not integration_tests_d" --junitxml pytest.xml tests/integration_tests
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, this is because tests for groups a/b/c/d, are explicitly marked with pytest.mark while the last bucket (in this case "e"), is a catch-all for all of the rest of unmarked tests. The thinking here was that folks adding a new file in integration_tests/ shouldn't have to think about assigning a marker.

- "integration_tests_b"
- "integration_tests_c"
- "integration_tests_d"
- "integration_tests_e"
Copy link
Contributor

Choose a reason for hiding this comment

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

This should work if we want to keep the markers unchanged:

# catch-all
- "not integration_tests_a and not integration_tests_b and not integration_tests_c and not integration_tests_d"

@tgaddair tgaddair merged commit c52bb8e into master Sep 16, 2023
@tgaddair tgaddair deleted the ref-tests branch September 16, 2023 21:09
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