Skip to content

Trying to fix tests on windows #93

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

Merged
merged 13 commits into from
Sep 5, 2021

Conversation

alexmojaki
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Sep 3, 2021

Codecov Report

Merging #93 (453eabd) into master (66eaa80) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master      #93   +/-   ##
=======================================
  Coverage   99.60%   99.61%           
=======================================
  Files           7        7           
  Lines         506      513    +7     
  Branches       71       71           
=======================================
+ Hits          504      511    +7     
  Misses          2        2           
Impacted Files Coverage Δ
devtools/debug.py 100.00% <100.00%> (ø)
devtools/prettier.py 100.00% <0.00%> (ø)
devtools/utils.py 96.29% <0.00%> (+0.74%) ⬆️

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 66eaa80...453eabd. Read the comment docs.

Copy link
Owner

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

otherwise this looks great.

@samuelcolvin
Copy link
Owner

just noticed some tests are failing on windows, but CI is passing. We had the same problem on pydantic and discovered that CI setup like

    - name: test with extras
      run: |
        make test
        coverage xml

when run with windows doesn't detect a failure in the first command.

I'll try to fix or skip these tests since they aren't really related to what you were fixing here (easier to fix here than on main since you've resolved a bunch of problems with windows 🙏)

@alexmojaki
Copy link
Contributor Author

There's an actual failure in executing on Windows 3.9+. This is annoying and worrying. I've never seen executing affected by the OS. Any idea why it would matter? Isn't the bytecode the same?

@samuelcolvin
Copy link
Owner

Yes, just saw it.

I have absolutely no idea why this combination would fail. I also don't have a windows machine to test with.

Shall I leave this with you? I'm happy to skip or xfail this test on these os/version combinationd, but I understand you might want to look into it.

@alexmojaki
Copy link
Contributor Author

I definitely need to investigate but for now please just xfail it.

@samuelcolvin samuelcolvin merged commit bb413fb into samuelcolvin:master Sep 5, 2021
@samuelcolvin
Copy link
Owner

thanks so much.

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.

2 participants