-
Notifications
You must be signed in to change notification settings - Fork 65
Update test matrix #403
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
Update test matrix #403
Conversation
.github/workflows/Test.yml
Outdated
| # * Windows using windows-latest | ||
| os: ['ubuntu-latest', 'windows-latest'] | ||
| omc-version: ['stable', 'nightly'] | ||
| # * OM 1.25.0 - before changing definition of simulation overrides |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We really don't want to test a specific OM version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we? I can remove it; however, at this point we have a hard to find break due to the handling of the simulation related options. My idea was to check / verify this for a given time. Please also see the comment by @joewa about the use of older versions (#401 (comment))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK; I scaled it down to 2x Python; 2x OS; 2x OM - the minimum number to test
.github/workflows/Test.yml
Outdated
| # * OM stable - latest stable version | ||
| # * OM nightly - latest nightly build | ||
| omc-version: ['1.25.0', 'stable', 'nightly'] | ||
| # => total of 12 runs for each test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be 18 tests in total.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed!
python-version: ['3.10', '3.12', '3.14'] os: ['ubuntu-latest', 'windows-latest'] omc-version: ['1.25.0', 'stable', 'nightly']
.github/workflows/Test.yml
Outdated
| # * OM stable - latest stable version | ||
| # * OM nightly - latest nightly build | ||
| omc-version: ['1.25.0', 'stable', 'nightly'] | ||
| # => total of 3 (Python) * 2 (OS) * 3 (OM) = 19 runs for each test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
18 runs for each test.
This is additional information that will quickly become outdated once a new Python version is added. I suggest not including this line, as it is easy to forget updating this part when other parts of the file are modified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
quite right! - I did remove the line ...
Based on PR #400 / #402 the test matrix needs to check for different settings including a breaking change in OM;