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

Separate slow tests from quick ones #758

Merged
merged 7 commits into from Sep 25, 2023

Conversation

matteobachetti
Copy link
Member

@matteobachetti matteobachetti commented Sep 22, 2023

@pupperemeritus taught me that there is a nice option in pytest, that allows to mark tests as slow and avoid their execution. Here, I'm separating the execution of quick tests from the slow ones, in two ways:

  • By marking single tests with @pytest.mark.slow
  • By marking whole test modules with pytestmark = pytest.mark.slow.

To execute only the slow tests, it's sufficient to use the options --run-slow -m (the first option comes from pytest-astropy)
This way, we can execute only unmarked tests in most test suites, and leave only one/two with the full tests. This can give much quicker feedback if a PR breaks something.

@codecov
Copy link

codecov bot commented Sep 22, 2023

Codecov Report

Merging #758 (67087e3) into main (e3acafe) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #758   +/-   ##
=======================================
  Coverage   97.25%   97.25%           
=======================================
  Files          41       41           
  Lines        7387     7388    +1     
=======================================
+ Hits         7184     7185    +1     
  Misses        203      203           
Files Changed Coverage Δ
stingray/pulse/overlapandsave/test_ols.py 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@dhuppenkothen dhuppenkothen left a comment

Choose a reason for hiding this comment

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

Seems like a great idea! I think I don't quite understand how this changes our test strategy, but in principle, I'm all for it! :)

@@ -25,6 +25,9 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
with:
ref: ${{ github.event.pull_request.head.ref }}
repository: ${{github.event.pull_request.head.repo.full_name}}
Copy link
Member

Choose a reason for hiding this comment

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

What does this do?

Copy link
Member Author

Choose a reason for hiding this comment

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

This one fixes a problem we had with PRs from forks, while maintaining consistency in catching the correct commit message. There was some issue with the commit message, that sometimes was the correct one and sometimes the merge message (when github makes the merge with the current main before testing). Now it looks robust enough (I'm doing this PR from a fork to prove it)

.github/workflows/ci_test.yml Show resolved Hide resolved
tox_env: 'py311-test-cov'
use_remote_data: true
experimental: false
slow: true
Copy link
Member

Choose a reason for hiding this comment

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

So does this take the previous matrix of operating systems and environments, and basically just writes them out individually? Does this mean we're no longer testing python 3.9, for example, and we're only testing Windows and MacOS on Python 3.11?

Copy link
Member Author

Choose a reason for hiding this comment

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

Our matrix was never an actual matrix, but always a list of single tests, as far as I remember. At this moment, indeed, we are testing with py38, py310 and py311. I couldn't manage to make py312-dev work, so left it out. Would you prefer to also have py39?

@matteobachetti matteobachetti added this pull request to the merge queue Sep 25, 2023
Merged via the queue into StingraySoftware:main with commit 5fac201 Sep 25, 2023
15 checks passed
@matteobachetti matteobachetti deleted the slow_tests branch September 25, 2023 13:27
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.

None yet

2 participants