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

Fix YAML/Cython tests #664

Merged
merged 6 commits into from
Jul 23, 2019
Merged

Fix YAML/Cython tests #664

merged 6 commits into from
Jul 23, 2019

Conversation

bryanwweber
Copy link
Member

@bryanwweber bryanwweber commented Jun 28, 2019 •
edited
Loading

These changes fix a few things that broke when testing the installed Cython interface (via python -m unittest -v cantera.test)

Changes proposed in this pull request:

  • Install the test/data/test_subdir in the Cython interface
  • Avoid using paths in the test files that implicitly relied on being in the source tree
  • Write test conversions into the test_work_dir instead of the cwd

@speth
Copy link
Member

speth commented Jun 28, 2019

Don't forget to rebase -i so that your fixup commit gets squashed correctly (I'm assuming GitHub doesn't do autosquash).

@bryanwweber
Copy link
Member Author

Don't forget to rebase -i so that your fixup commit gets squashed correctly (I'm assuming GitHub doesn't do autosquash).

Once the changes are approved, I'll autosquash and then it can be merged.

@codecov
Copy link

codecov bot commented Jun 29, 2019

Codecov Report

Merging #664 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #664      +/-   ##
==========================================
+ Coverage   70.78%   70.78%   +<.01%     
==========================================
  Files         373      373              
  Lines       43454    43454              
==========================================
+ Hits        30757    30758       +1     
+ Misses      12697    12696       -1
Impacted Files Coverage Δ
src/transport/GasTransport.cpp 90.58% <0%> (+0.2%) ⬆️

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 c840142...93a8deb. Read the comment docs.

@bryanwweber
[Cython/Test] Install test subdirs in Cython interface
a4ad2f5 
The tests now include subdirectories with data files, so those should be
installed with the rest of the data files
@bryanwweber
[Test] Avoid using path in source tree in tests
1e2a9f2 
The relative path that is eliminated here relied on being in the source
directory structure, breaking tests of the installed Cython interface
@bryanwweber
[Test] Remove relative directory for data files
2e54811 
The relative directory wasn't the same for the installed Cython
interface
@bryanwweber
[Test] Write converted test files to the work dir
8d57424 
The previous behavior was to write the test files to the current working
directory
@bryanwweber
[CTI/YAML] Change cti2yaml to use pathlib
2743875 
cti2yaml converts input and output file names to pathlib.Path objects.
This makes it easier to manage paths for test data files
@bryanwweber
[CTI/YAML] Check convert arguments in cti2yaml
5f08b36 
Check that one, and only one, of the filename/text arguments are
specified.
@codecov
Copy link

codecov bot commented Jun 29, 2019

Codecov Report

Merging #664 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #664      +/-   ##
==========================================
- Coverage   70.78%   70.78%   -0.01%     
==========================================
  Files         373      373              
  Lines       43454    43454              
==========================================
- Hits        30758    30757       -1     
- Misses      12696    12697       +1
Impacted Files Coverage Δ
src/transport/GasTransport.cpp 90.37% <0%> (-0.21%) ⬇️

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 674c37c...5f08b36. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Jun 29, 2019 •
edited
Loading

Codecov Report

Merging #664 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #664      +/-   ##
==========================================
- Coverage   70.78%   70.78%   -0.01%     
==========================================
  Files         373      373              
  Lines       43454    43454              
==========================================
- Hits        30758    30757       -1     
- Misses      12696    12697       +1
Impacted Files Coverage Δ
src/transport/GasTransport.cpp 90.37% <0%> (-0.21%) ⬇️

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 674c37c...5f08b36. Read the comment docs.

@bryanwweber
Copy link
Member Author

This is rebased and squashed now. 👍?

@bryanwweber
Copy link
Member Author

bryanwweber commented Jul 21, 2019 •
edited
Loading

I'll merge this tomorrow if no further comments. This is blocking for Cantera/conda-recipes#19

@bryanwweber bryanwweber merged commit 5f08b36 into Cantera:master Jul 23, 2019
@bryanwweber bryanwweber deleted the fix_yaml_cython_tests branch November 26, 2019 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@speth speth speth 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.
2 participants
@bryanwweber @speth