Skip to content

Add "validation" pytest marker#2633

Merged
VeckoTheGecko merged 6 commits into
Parcels-code:mainfrom
VeckoTheGecko:push-yklynlyqxsrr
May 18, 2026
Merged

Add "validation" pytest marker#2633
VeckoTheGecko merged 6 commits into
Parcels-code:mainfrom
VeckoTheGecko:push-yklynlyqxsrr

Conversation

@VeckoTheGecko
Copy link
Copy Markdown
Contributor

@VeckoTheGecko VeckoTheGecko commented May 18, 2026

Description

This PR:

  • Adds a validation marker for the validation tests
  • Moves the validation tests (so that they can now be picked up by pytest - pytest assumes a file name of test_...)
  • Updates the test finding to skip flaky and validation tests by default (also updates our CI to match)
  • Adds test-flaky and test-validation pixi tasks

Checklist

AI Disclosure

None used

@VeckoTheGecko
Copy link
Copy Markdown
Contributor Author

@erikvansebille @fluidnumericsJoe do you think we should add another github action workflow which can run the validation tests?

Options:

  • have it run all the time
  • have it run on request (i.e., when a "run-validation" label is applied to a PR, then the workflow will be run for that PR)
  • leave it to be run manually by the user

@VeckoTheGecko VeckoTheGecko review requested due to automatic review settings May 18, 2026 09:51
Copy link
Copy Markdown
Member

@erikvansebille erikvansebille 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; two small comments below

AdvectionRK4_3D,
)

pytestmark = pytest.mark.validation
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is this needed? Does not seem to be used in the rest of the script?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is used by Pytest (pytest is a mildly magical package in the sense that it looks for these sorts of specially named things). It marks the whole module as validation

You can see that by doing pytest -m validation

Comment thread pyproject.toml Outdated
@erikvansebille
Copy link
Copy Markdown
Member

@erikvansebille @fluidnumericsJoe do you think we should add another github action workflow which can run the validation tests?

Options:

  • have it run all the time
  • have it run on request (i.e., when a "run-validation" label is applied to a PR, then the workflow will be run for that PR)
  • leave it to be run manually by the user

We could run the validation before we do a release? Perhaps simply by adding it to our own workflow notes?

@VeckoTheGecko
Copy link
Copy Markdown
Contributor Author

We could run the validation before we do a release? Perhaps simply by adding it to our own workflow notes?

I think that sounds good

@VeckoTheGecko VeckoTheGecko merged commit 119256a into Parcels-code:main May 18, 2026
15 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in Parcels development May 18, 2026
@VeckoTheGecko VeckoTheGecko deleted the push-yklynlyqxsrr branch May 18, 2026 11:09
@fluidnumericsJoe
Copy link
Copy Markdown
Contributor

Why would we skip them when other tests are run? Feels like a recipe to potentially miss things until we want to release and there's "one more thing" to do at release. If there's tests that fail then it could kill predictability here on time to release

@fluidnumericsJoe
Copy link
Copy Markdown
Contributor

IMO the validation tests should be run on PRs and pushes to main. We previously had code that was "correct" but wasn't found out as problematic until these examples were run out of cycle in python notebooks; hence why they were brought in.

@VeckoTheGecko
Copy link
Copy Markdown
Contributor Author

Gotcha. Thanks for the input @fluidnumericsJoe . Since CI times were a concern last time we chatted, would it be useful to have it as another required workflow in GHA thats separate to the unit tests? Would that be a good solution?

@VeckoTheGecko
Copy link
Copy Markdown
Contributor Author

And I think you're right @fluidnumericsJoe - having this in the iteration cycle in CI is a good idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants