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.

Sign up for GitHub

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

Jump to bottom

Collect build/test timing information and fix some bugs #1020

Merged
merged 15 commits into from
Apr 28, 2021
Merged

Collect build/test timing information and fix some bugs #1020

merged 15 commits into from
Apr 28, 2021
+99 −51

Conversation

speth
Copy link
Member

@speth speth commented Apr 25, 2021 •
edited
Loading

Changes proposed in this pull request

  • Use pytest to run Python tests if it is installed
  • Collect timing information for SCons commands and Python tests
  • Start coverage tests immediately, rather than waiting for Ubuntu tests to finish
  • Fix some diffusion flame extinction cases that weren't using the correct inlet temperature
  • Fix set_initial_guess for CounterFlowTwinPremixedFlame so it doesn't overwrite deliberately-set profiles
  • Disable the test_ignition_delay_sensitivity - it's a never-ending source of trouble, and the problems with it are already documented in Improve reactor network sensitivity analysis enhancements#55
  • Speed up compilation a bit by adding AnyMap.h to the precompiled header.

Note: I removed a bunch of changes from this PR that started to make material changes to the test suite, for the sake of getting something we can merge sooner and get back to having the CI servers working.

Checklist

  • There is a clear use-case for this code change
  • The commit message has a short title & references relevant issues
  • Build passes (scons build & scons test) and unit tests address code coverage
  • The pull request is ready for review

@speth speth force-pushed the use-pytest branch 7 times, most recently from a8fca78 to e963a94 Compare April 27, 2021 00:53
@speth speth closed this Apr 27, 2021
@speth speth reopened this Apr 27, 2021
@speth
Copy link
Member Author

speth commented Apr 27, 2021

Closed and re-opened in the hope that this would get the coverage report to actually show up.

@bryanwweber
Copy link
Member

@speth Coverage is not uploaded if the tests fail

@speth
Copy link
Member Author

speth commented Apr 27, 2021

Yes, I know that, but it was missing before after several force-pushes where all of the tests did succeed. Right now, I'm trying to get the annotations for failing tests generated by https://pypi.org/project/pytest-github-actions-annotate-failures/ to show up.

@speth
Copy link
Member Author

speth commented Apr 27, 2021

Well, the annotations show up here: https://github.com/Cantera/cantera/actions/runs/787611863, which is somewhat helpful, but they don't show up in the PR under "Files Changed". I'm guessing the reason is because pytest sees the test's path as something like ../../build/python/cantera/test/test_kinetics.py, rather than interfaces/cython/cantera/test/test_kinetics.py, and can't find the right file in the repository to annotate.

@speth speth changed the title Collect build/test timing information Collect build/test timing information and speed up CI tests Apr 27, 2021
@speth speth changed the title Collect build/test timing information and speed up CI tests Collect build/test timing information Apr 27, 2021
@speth speth changed the title Collect build/test timing information Collect build/test timing information and fix some bugs Apr 27, 2021
@speth speth requested a review from a team April 27, 2021 23:55
speth added 3 commits April 27, 2021 21:52
@speth
[Test] Skip test_ignition_delay_sensitivity
542532e 
This test is not reliable and causes lots of CI headaches.

Some of the problems are documented in
Cantera/enhancements#55
@speth
[CI] Use all three CPUs available on macOS builders
4ac8f67
@speth
[SCons] Add AnyMap.h to precompiled header
c6b2151 
Speeds up overall compilation time because this header and all the
templated code in AnyMap.inl.h is directly or indirectly included in
many other files.
ischoegl
ischoegl reviewed Apr 28, 2021
View reviewed changes
Copy link
Member

@ischoegl ischoegl left a comment

Choose a reason for hiding this comment

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

I believe this looks good.

ischoegl
ischoegl requested changes Apr 28, 2021
View reviewed changes
Copy link
Member

@ischoegl ischoegl left a comment

Choose a reason for hiding this comment

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

I guess I did find something after all.

@speth speth requested a review from ischoegl April 28, 2021 20:17
@speth speth merged commit ddb394b into Cantera:main Apr 28, 2021
This was referenced Apr 29, 2021
Closed
Merged
@speth speth deleted the use-pytest branch May 9, 2021 02:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ischoegl ischoegl ischoegl approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@speth @bryanwweber @ischoegl