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

Improve kernel tests #867

Open
wants to merge 2 commits into
base: devel
Choose a base branch
from

Conversation

bobturneruk
Copy link
Contributor

@bobturneruk bobturneruk commented Sep 28, 2020

Kernel tests are executed against each kernel using the check_kernel_gradient_functions function which, in itself, calls and executes a number of tests. Currently, if one of these tests fails or errors, the subsequent tests do not run. This PR modifies the code such that if any one test fails or errors, a failure is reported, but the additional tests still run. This means that a developer can fix problems causing tests to fail in any order they like.

Copy link

@drj11 drj11 left a comment

Choose a reason for hiding this comment

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

sure, it seems reasonable for now.

Really it seems to me that these tests should be separated so that the test framework can "see" them individually and run and report separately.

Possibly using parameterized tests: https://docs.pytest.org/en/stable/parametrize.html#basic-pytest-generate-tests-example

Tests that can't succeed should be skipped: https://docs.pytest.org/en/stable/skipping.html

I'm not suggesting you do that in this PR, it's another PR. But note that doing either of things i suggest above involves really pinning down which test framework you want to go forward with.

@bobturneruk
Copy link
Contributor Author

Thanks @drj11 - I completely agree, it's (hopefully) an improvement, but not ideal. If the resources are available, I'd favour moving from nosetests to pytest as the latter has more current support and is likely to have more longevity.

@bobturneruk bobturneruk marked this pull request as ready for review September 29, 2020 10:30
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