Skip to content

Add unit tests and continuous integration#3

Merged
xylar merged 8 commits into
ismip:mainfrom
xylar:add-ci-and-unit-tests
May 7, 2026
Merged

Add unit tests and continuous integration#3
xylar merged 8 commits into
ismip:mainfrom
xylar:add-ci-and-unit-tests

Conversation

@xylar
Copy link
Copy Markdown
Member

@xylar xylar commented May 1, 2026

These tests require some small changes to the checker and the generator so it can run in a temp directory.

They test the different expected failure modes of the checker (as well as successes).

They should run automatically with pushes and pull requests.

@xylar xylar added the enhancement New feature or request label May 1, 2026
@xylar xylar requested a review from hgoelzer May 1, 2026 18:24
@xylar
Copy link
Copy Markdown
Member Author

xylar commented May 1, 2026

Unfortunately, I don't think CI will run until after this goes in.

@xylar
Copy link
Copy Markdown
Member Author

xylar commented May 1, 2026

$ conda activate isschecker
(isschecker) xylar@katara:~/code/ISM_SimulationChecker/add-ci-and-unit-tests$ pytest tests
============================= test session starts ==============================
platform linux -- Python 3.14.4, pytest-9.0.3, pluggy-1.6.0
rootdir: /home/xylar/code/ISM_SimulationChecker/add-ci-and-unit-tests
collected 6 items                                                              

tests/test_compliance_checker.py ......                                  [100%]

============================== 6 passed in 1.04s ===============================

Works locally for me at least.

@hgoelzer
Copy link
Copy Markdown
Member

hgoelzer commented May 4, 2026

Hi, this looks like a good addition. I hopefully can look at it in more detail soon. But here already one top-level comment.
It may be that 'tests' is the default location for this, but having both 'test' and 'tests' seems a bit confusing on first look. Any chance we can resolve that. If 'tests' is indeed the standard, we may rename 'test' to 'generate' or similar.

@xylar
Copy link
Copy Markdown
Member Author

xylar commented May 5, 2026

Good call, @hgoelzer. I was struggling with test and tests. The latter is, indeed, a standard name but I was going to rename it unit_tests. But I like your idea of renaming test to generate better so I've done that.

@xylar xylar force-pushed the add-ci-and-unit-tests branch from 1eca0bb to 6fab4ec Compare May 5, 2026 11:22
@xylar
Copy link
Copy Markdown
Member Author

xylar commented May 5, 2026

Tests aren't running here in the pull request yet but they are running on my fork:
https://github.com/xylar/ISM_SimulationChecker/actions/runs/25373388060/job/74402245575

Copy link
Copy Markdown
Member

@hgoelzer hgoelzer left a comment

Choose a reason for hiding this comment

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

Looks good and works for me. After running pytest I was wondering where the test netcdf files were and realised they are created on the fly and deleted again, in line with an automated test procedure. I've learned that the files can be retained and inspected by adding --basetemp to the call, like so
conda run -n isschecker pytest -v tests/test_compliance_checker.py --basetemp=/tmp/pytest_tmp
This could be added to the README for transparency, unless this is just my curiosity and not relevant for our standard use case. I leave that judgement up to you. Great to have this feature in. Thanks

Added instructions for retaining test files during pytest.
@xylar
Copy link
Copy Markdown
Member Author

xylar commented May 7, 2026

@hgoelzer, --basetemp isn't a feature I've used before but it looks like it could be useful. I added it to the README.

@xylar xylar merged commit 1bd2167 into ismip:main May 7, 2026
@xylar xylar deleted the add-ci-and-unit-tests branch May 7, 2026 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants