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

Fix some typos/missing words #81

Merged
merged 1 commit into from
Mar 11, 2017
Merged

Conversation

brettcannon
Copy link
Contributor

No description provided.

@codecov-io
Copy link

Codecov Report

Merging #81 into master will decrease coverage by 0.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master      #81      +/-   ##
==========================================
- Coverage   98.29%   98.24%   -0.06%     
==========================================
  Files          50       50              
  Lines        5876     5876              
  Branches      464      464              
==========================================
- Hits         5776     5773       -3     
- Misses         86       88       +2     
- Partials       14       15       +1
Impacted Files Coverage Δ
trio/_core/_io_windows.py 75% <0%> (-1.75%)

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 08bf8b6...892ef62. Read the comment docs.

@dimaqq
Copy link

dimaqq commented Mar 11, 2017

Wow codecov is so confused!

@brettcannon
Copy link
Contributor Author

@dimaqq actually it's accurate since I did add lines to the project that didn't increase the coverage. I believe there's a way to say "ignore these directories" which would solve this.

@brettcannon
Copy link
Contributor Author

@njsmith FYI Codecov has a YAML file you can check in to configure it if you want help tweaking settings.

@njsmith
Copy link
Member

njsmith commented Mar 11, 2017

No, codecov doesn't care about README -- the problem is that there's a branch in _io_windows.py that gets taken non-deterministically, so there are 3 lines that are flopping between 2 covered + 1 uncovered versus 1 covered + 2 uncovered :-(.

@brettcannon
Copy link
Contributor Author

@njsmith have you thought about using # pragma: no cover for those non-deterministic lines?

@njsmith
Copy link
Member

njsmith commented Mar 11, 2017

It's a legitimate complaint though :-) We should have coverage for both branches, just I got lazy about writing tests for a few bits of code that aren't actually used yet, and are just placeholders for future expansion. (Specifically the non-socket APIs for Windows and Kevent, which I figure will get sorted out properly once we add subprocess support - see also #4, #26, #52.) I have a patch here that I think makes the test coverage deterministic at least though...

njsmith added a commit to njsmith/trio that referenced this pull request Mar 11, 2017
There's a race condition when we signal the IOCP thread to exit by
closing the IOCP: the thread might be blocked in
GetQueuedCompletionStatusEx, or it might (rarely) be in the rest of
the loop. The effect of this is that there are two possible error
messages we might get that both indicate that the IOCP was
closed. Before we were only detecting one of them; now we detect
both.

This has the side-effect of making it so that during the test suite we
deterministically only take the regular-exit path, rather than
sometimes taking the blow-up-on-unrecognized-error path. Which is
good because codecov was getting cranky about that non-deterministic
coverage and yelling at people for no reason (see python-triogh-81).
@njsmith njsmith merged commit e169547 into python-trio:master Mar 11, 2017
@brettcannon brettcannon deleted the patch-1 branch March 11, 2017 22:44
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.

4 participants