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

CI: Add script for running tests from github worklow #272

Merged
merged 6 commits into from
Jan 28, 2025

Conversation

slangeveld
Copy link
Contributor

@slangeveld slangeveld commented Jan 23, 2025

Resolves part of https://github.com/equinor/rms-sys/issues/267

Added a bash script that can be run from the github workflows to execute the fmu-tools unit tests towards an RMS Python Environment. The script copies the required test files, installs the test dependencies and runs the pytests. Note that the xtgeo-testdata files needed to run the tests are cached on the github actions runner som pointing to the path XTGEO_TESTDATA_PATH with the cached data should be sufficient to run the tests.

Tests have been updated to be able to use the testdata path XTGEO_TESTDATA_PATH set in the test config:

  • Since the tests are to be run from a github workflow, the tests need to support xtgeo-testdata being placed at the path given by XTGEO_TESTDATA_PATH in the command pytest ./tests -n 4 -vv --testdatapath $XTGEO_TESTDATA_PATH.

A testrun from can be seen here https://github.com/equinor/rms-sys/actions/runs/13006244695/job/36273691991

@slangeveld slangeveld marked this pull request as ready for review January 23, 2025 13:08
@slangeveld slangeveld requested a review from mferrera January 23, 2025 13:17
ci/testrmsenv.sh Outdated Show resolved Hide resolved
Copy link
Collaborator

@mferrera mferrera 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 👍

For posterity there was some offline discussion about this

ci/testrmsenv.sh Outdated
run_pytest () {
echo "Running fmu-tools tests with pytest..."
pushd $CI_TEST_ROOT
pytest -n 4 -vv -m "not skipunlessroxar"
Copy link
Collaborator

Choose a reason for hiding this comment

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

My brain can't boolean today, but I think we want to include the the roxar tests (does this do that)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The intention here was to exclude it, to avoid calling the RMS API to often as discussed here https://github.com/equinor/rms-sys/pull/254

But open to include it if we want that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Saw your comment here equinor/xtgeo#1297

Will include the roxar tests 👍

ci/testrmsenv.sh Outdated
Comment on lines 35 to 36
echo "Installing test dependencies..."
pip install --upgrade --force-reinstall ".[tests]"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Future selves shall thank us for an isolated install 👍

@slangeveld slangeveld force-pushed the 267-add-test-rmsenv-script branch from 66e6924 to fd674d2 Compare January 23, 2025 14:01
Copy link
Collaborator

@mferrera mferrera left a comment

Choose a reason for hiding this comment

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

Nice work 👍

@slangeveld slangeveld merged commit 9788a6c into main Jan 28, 2025
8 checks passed
@slangeveld slangeveld deleted the 267-add-test-rmsenv-script branch January 28, 2025 12:35
slangeveld added a commit that referenced this pull request Feb 3, 2025
* CI: Add script for running tests from github worklow (#271)

* CI: Install dependencies for `tests`

* CI: Clone xtgeo testdata for tests

* CI: Include roxar tests

* CI: Use xtgeo-testdata from cache

* TST: Use testdata path from test config
slangeveld added a commit that referenced this pull request Feb 11, 2025
* CI: Add script for running tests from github worklow (#272)

* CI: Add script for running tests from github worklow (#271)

* CI: Install dependencies for `tests`

* CI: Clone xtgeo testdata for tests

* CI: Include roxar tests

* CI: Use xtgeo-testdata from cache

* TST: Use testdata path from test config

* TST: Tweak depth convert numerical stability

One test case was commented out and this solution may not be robust on
all machines.

---------

Co-authored-by: mferrera <[email protected]>
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