Skip to content

Test examples#136

Draft
maxnus wants to merge 9 commits intoBoothGroup:masterfrom
maxnus:test_examples
Draft

Test examples#136
maxnus wants to merge 9 commits intoBoothGroup:masterfrom
maxnus:test_examples

Conversation

@maxnus
Copy link
Contributor

@maxnus maxnus commented Sep 20, 2023

I think we currently don't test that all the examples run without exception?
This adds tests for all example files.

@obackhouse
Copy link
Contributor

I'm generally against this in the CI because examples should include larger examples with possibly longer runtimes and might (now or in the future) butcher the test runtime

@basilib
Copy link
Contributor

basilib commented Sep 21, 2023

There's a script in the examples folder to run them and report any exceptions.

@obackhouse
Copy link
Contributor

yeah the CI on this PR hit the maximum 6 hours for a single job (you can have longer workflows, they just have to consist of sequential jobs <6h I believe). Even if we get the examples down to 6 hours it's not an option for the push-triggered or nightly runs.

@maxnus
Copy link
Contributor Author

maxnus commented Sep 21, 2023

I'm generally against this in the CI because examples should include larger examples with possibly longer runtimes and might (now or in the future) butcher the test runtime

That's fair, we can take it out of the CI tests. Or, perhaps better, just blacklist the example we want to skip. I think we do something like that already for tests which are marked very slow?

As a side note, I think we should really aim for examples with < 1min runtime. Even for some self-consistent examples, we should just increase the tolerance such that it only run 2 or 3 iterations - it's only about demonstrating the concept, not to give useful results.

@maxnus maxnus marked this pull request as draft September 21, 2023 18:15
@maxnus
Copy link
Contributor Author

maxnus commented Sep 21, 2023

There's a script in the examples folder to run them and report any exceptions.

I didn't see that! To be honest, I don't think it should be there. It's not very useful for a user, it's more like a test - and if it's a test it should be in the test directory and use pytest syntax, like all other tests.

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.

3 participants