Drop unittest for integration and system tests #949
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
launch jenkins all balfrin tsa |
daint🟢 unit test
|
balfrin🟢 unit test
🟢 integration test
🔴 system test
WARNING: Serial tests did not run for system tests |
|
|
||
| def test_tensorflowc(self): | ||
| spack_spec('tensorflowc') | ||
| @pytest.mark.infero |
There was a problem hiding this comment.
What happens if the marker is missing?
Because I think it will only be a matter of time until someone forgets one.
There was a problem hiding this comment.
If the marker is missing the test is not in a give scope, so it is not executed
Comment on lines
+75
to
+79
| @pytest.mark.cosmo | ||
| @pytest.mark.parametrize( | ||
| 'variant', ['cosmo_target=gpu ~cppdycore', 'cosmo_target=cpu ~cppdycore']) | ||
| def test_cosmo_spec_with_variant(variant: str): | ||
| spack_spec(f'cosmo {variant}') |
There was a problem hiding this comment.
Why does the cosmo test have a cosmo marker? That carries so little information.
There was a problem hiding this comment.
Since we don't rely on parsing function/clasee-names anymore conditional tests need a marker.
I agree this is a bit too verbose, but at least it is bullet-proof.
There was a problem hiding this comment.
But prone to be forgotten, misspelled or wrong because of a copy-past mistake.
There was a problem hiding this comment.
That's what review are for ;-)
An other option would be to create the marker on the fly based on the function name, itself prone to be forgotten or misspelled.
I went for that approach to also simplify the log-filename creation.
dominichofer
approved these changes
May 22, 2024
|
launch jenkins balfrin daint all |
balfrin🟢 unit test
🔴 integration test
WARNING: Serial tests did not run for system tests |
daint🟢 unit test
🔴 integration test
WARNING: Serial tests did not run for system tests |
|
launch jenkins all daint balfrin |
daint🟢 unit test
🟢 integration test
🔴 system test
WARNING: Serial tests did not run for system tests |
balfrin🟢 unit test
🟢 integration test
🔴 system test
WARNING: Serial tests did not run for system tests |
|
launch jenkins daint balfrin py-gt4py infero |
balfrin🟢 unit test
🟢 integration test
|
daint🟢 unit test
🟢 integration test
🔴 system test
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replace unittest with pure pytest, see #945
Additionally
devirtualize_envbecame a pytest.fixtures and is explicitly added to all relevant tests.