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 spec directory name of FHDLTestCase #511

Merged
merged 1 commit into from Oct 22, 2020

Conversation

anuejn
Copy link
Contributor

@anuejn anuejn commented Oct 22, 2020

This PR improves the internal version of FHDLTestCase by trying to get the last stack frame that is still inside the nmigen codebase when creating names for the spec file. This helps to make the FIFO test cases produce all distinct spec directories and makes the nmigen test suite thread-safe (can be run successfully with pytest-xdist)

@whitequark
Copy link
Member

This is kind of cursed, but... the whole point of making FHDLTestCase internal is that we can do whatever cursed stuff we want. So I'm fine with it.

@whitequark
Copy link
Member

trying to get the last stack frame that is still inside the nmigen codebase when creating names for the spec file

Wait, how does this actually work?

@codecov
Copy link

codecov bot commented Oct 22, 2020

Codecov Report

Merging #511 into master will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #511   +/-   ##
=======================================
  Coverage   81.33%   81.34%           
=======================================
  Files          49       49           
  Lines        6409     6412    +3     
  Branches     1278     1279    +1     
=======================================
+ Hits         5213     5216    +3     
  Misses       1007     1007           
  Partials      189      189           
Impacted Files Coverage Δ
nmigen/sim/_pyrtl.py 95.20% <0.00%> (+0.04%) ⬆️

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 2c505de...bc90456. Read the comment docs.

@whitequark
Copy link
Member

Ah okay, I see how it works. I think you shouldn't match nmigen in the path because there's no guarantee or requirement for the git checkout name; in particular I think CI often check out the project as something like ~/work (Appveyor does it iirc?). Maybe match against dirname of utils.py?

@anuejn
Copy link
Contributor Author

anuejn commented Oct 22, 2020

Ah you are right, I missed that. Sounds good; will do :)

@whitequark
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants