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

using pytest command instead of bash #72

Merged
merged 15 commits into from
Aug 26, 2021

Conversation

shimwell
Copy link
Member

Proposed changes

Following on from the improvments to the CI that @RemDelaporteMathurin made I am keen to rewrite on of the actions.

Previously this action didn't fail even when the tests failed.

I think is is due to running the tests via a bash script.

So this PR runs the tests directly with a pytest command

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Code refactoring
  • Documentation Update (if none of the other choices apply)
  • New tests

Checklist

  • Pep8 applied
  • Unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@shimwell
Copy link
Member Author

Interestingly the improved GitHub actions CI appears to have caught an error that appears to pass on CircleCI

@codecov
Copy link

codecov bot commented Aug 25, 2021

Codecov Report

Merging #72 (746863f) into develop (6dc9ef2) will increase coverage by 0.18%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #72      +/-   ##
===========================================
+ Coverage    97.72%   97.90%   +0.18%     
===========================================
  Files           75       75              
  Lines         5096     4870     -226     
===========================================
- Hits          4980     4768     -212     
+ Misses         116      102      -14     
Impacted Files Coverage Δ
paramak/__init__.py 100.00% <ø> (ø)
paramak/parametric_reactors/ball_reactor.py 98.91% <ø> (-0.01%) ⬇️
...parametric_reactors/center_column_study_reactor.py 100.00% <ø> (ø)
...aramak/parametric_reactors/eu_demo_2015_reactor.py 100.00% <ø> (ø)
paramak/parametric_reactors/submersion_reactor.py 98.63% <ø> (-0.01%) ⬇️
paramak/utils.py 97.30% <ø> (+4.55%) ⬆️
paramak/reactor.py 91.95% <100.00%> (-0.95%) ⬇️
paramak/shape.py 96.44% <100.00%> (-1.10%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6dc9ef2...746863f. Read the comment docs.

@RemDelaporteMathurin
Copy link
Member

@shimwell the commit e87c91f adds the action provided by Codecov for GA which successfully uploaded to codecov, fixing #23 .
However, the tests still don't pass 😄

@RemDelaporteMathurin RemDelaporteMathurin linked an issue Aug 26, 2021 that may be closed by this pull request
@shimwell
Copy link
Member Author

Yep it appears to be failing on the first use of Cubit.

There is a problem combining use of pymoab and cubit in the same script as they both point to different hdf5 libraries

Here is a description of that problem https://forum.coreform.com/t/conflicting-hdf5-libraries/1237/3?u=jshimwell

So there are four difficulties we have had with containerising cubit,

  • getting the lic file into the right folder /root/.config/Coreform/licenses/
  • having the correct filename name (must be cubit-learn.lic)
  • getting the contents of the lic file correct . should be a date Fri May 28 2021
  • avoiding the pymoab hdf5 conflict by not using pymoab and cubit in the same script

I guess the problem in the tests is the last one but tricky to check as no output is printed (even with verbose options turned on)

@RemDelaporteMathurin
Copy link
Member

And it's not failing with Circle

@shimwell
Copy link
Member Author

It also passes when I download the docker and run locally.

@shimwell
Copy link
Member Author

Ok I think I've got a solution for all of this. It is a bit radical but let me know what you think

I shall write it up in an issue #75

@shimwell shimwell merged commit 60a0af4 into develop Aug 26, 2021
@shimwell shimwell deleted the fixing_gh_action_to_permit_fails branch August 26, 2021 21:06
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.

GitHub based CI not uploading to codecov
2 participants