Skip to content

45 only run the unit tests which touch changed code in the CI #50

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 43 commits into from
Closed

45 only run the unit tests which touch changed code in the CI #50

wants to merge 43 commits into from

Conversation

@max-models
Copy link
Member

@max-models max-models commented Oct 24, 2025 •
edited
Loading

Solves the following issue(s):

Closes #45

Core changes:

None

Model-specific changes:

None

Documentation changes:

None

@max-models max-models linked an issue Oct 24, 2025 that may be closed by this pull request
Only run the unit tests which touch changed code in the CI #45
Closed
@max-models
Copy link
Member Author

max-models commented Oct 26, 2025 •
edited
Loading

It seems like the -k flag isn't working well with --testmon. When running the unit tests twice it still runs all of them unless I remove the -k flag. One option would be to categorize the tests in separate subdirectories instead and use --ignore=<PATH> to specify whether to run unit tests or model tests. I think it would be nice to organize the tests in src/struphy/tests with some subdirectories

@max-models
Continue if no testmondata is found
03adb04
@max-models
Added struphy_verification_tests/ to .gitignore
20f978d
@max-models
--ignore=model/tests
6343bf3
@max-models
Check testmon from devel before any other branch.
d5f638b
@max-models
Added TESTMON_DATAFILE to env
ca13950
@max-models
Added caching of .testmondata at the end of the pipeline
ac2b016
@max-models
Set concurrency with cancel-in-progress: true
e033901
@max-models
Moved verification tests into verification/
ab9e96e
@max-models
Specify tests with paths
0a72627
@max-models
FOrmatting
97ecf4c
@max-models
Moved one unit test
d201a74
@max-models
Formatting
4cf0c85
@max-models
Removed everything other than the unit test in the CI
768bce9
@max-models
Trigger CI
decfd63
@max-models
Moved model tests to tests/model
c69b28c
@max-models
Moved verification tests to tests/verification
c396007
@max-models
Moved all tests into tests/
0b82265
@max-models
Moved the last test
5cb8586
@max-models
Fixed incorrect import
6a5f232
@max-models
Removed duplicate testing
d9a02e9
@max-models
Cleanup test.py
91b8ab7
@max-models
Copy link
Member Author

max-models commented Oct 26, 2025

Seems like testmon might be problematic for MPI tests https://github.com/struphy-hub/struphy/actions/runs/18819880612/job/53694176303

max-models
max-models commented Oct 28, 2025
View reviewed changes
src/struphy/linear_algebra/saddle_point.py
from psydac.linalg.solvers import inverse

from struphy.linear_algebra.tests.test_saddlepoint_massmatrices import _plot_residual_norms
from struphy.tests.unit.linear_algebra.test_saddlepoint_massmatrices import _plot_residual_norms
Copy link
Member Author

@max-models max-models Oct 28, 2025

Choose a reason for hiding this comment

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

I find it a bit odd to import a plotting function from a testing file, should it not be the opposite way around?

Copy link
Member

@spossann spossann Oct 29, 2025

Choose a reason for hiding this comment

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

Yes i agree, please correct.

@spossann
Copy link
Member

spossann commented Oct 29, 2025

It seems like the -k flag isn't working well with --testmon. When running the unit tests twice it still runs all of them unless I remove the -k flag. One option would be to categorize the tests in separate subdirectories instead and use --ignore=<PATH> to specify whether to run unit tests or model tests. I think it would be nice to organize the tests in src/struphy/tests with some subdirectories

We can remove the -k flag, move test_models.py and all test_verif_.py to a separate folder and use --ignore. I am against collecting all tests in one struphy/tests/ folder if not really necessary; it is much clearer when the test are next to the code they test.

@max-models max-models mentioned this pull request Oct 29, 2025
Merged
@spossann
Copy link
Member

spossann commented Oct 29, 2025

Seems like testmon might be problematic for MPI tests https://github.com/struphy-hub/struphy/actions/runs/18819880612/job/53694176303

If this problem persists, we can try to shorten the mpi unit tests (using pytest.mark.mpi), for example by replacing the long convergence tests by simpler non-parameter-scan tests.

@max-models
Copy link
Member Author

max-models commented Oct 29, 2025

Seems like testmon might be problematic for MPI tests https://github.com/struphy-hub/struphy/actions/runs/18819880612/job/53694176303

If this problem persists, we can try to shorten the mpi unit tests (using pytest.mark.mpi), for example by replacing the long convergence tests by simpler non-parameter-scan tests.

This could be an option, but I think I can resolve it by only using --testmon to collect the tests which needs to be executed and then only running those, so we would do it in two steps: 1) filter test, 2) execute filtered tests. Now we do both of these in one go, and that seems not to work with --testmon.

@max-models max-models mentioned this pull request Oct 31, 2025
Merged
@max-models
Copy link
Member Author

max-models commented Oct 31, 2025

Moved to #96

@max-models max-models closed this Oct 31, 2025
spossann added a commit that referenced this pull request Nov 22, 2025
@max-models @spossann
Only run the unit tests which touch changed code in the ci (#96)
f5ce047 
Redo of #50

**Solves the following issue(s):**

Closes #45

**Core changes:**

There are now four PR workflows that must pass for a feature to be
merged:

1. unit
2. models
3. pure Python
4. with clones

Tests are made with and without MPI with `--testmon`. Testmon data is
cached for the next workflow run to use. Cache cannot be altered, thus
new cache keys are necessary in every run. The key convention is, for
unit tests for example:
```
key: testmon-unit-${{ github.event.number }}-${{ github.run_number }}
restore-keys: |
    testmon-unit-${{ github.event.number }}-
    testmon-unit-
``` 
The `event.number` is the number of the PR. The `run_number` refers to
the workflow run, thus it increases monotonically. The one that is
fetched has a run number one less than the current run, and is thus the
most recent.

---------

Co-authored-by: Stefan Possanner <[email protected]>
Co-authored-by: Stefan Possanner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@spossann spossann spossann left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Only run the unit tests which touch changed code in the CI

3 participants

@max-models @spossann