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

timeout coverage/pytest with sigint to help debug CI timeout issues #2827

Closed
wants to merge 5 commits into from

Conversation

jakkdl
Copy link
Member

@jakkdl jakkdl commented Oct 23, 2023

didn't find any dead simple ways of doing this within pytest. I hope this works on other platforms

@codecov
Copy link

codecov bot commented Oct 23, 2023

Codecov Report

Merging #2827 (ec87c56) into master (315a0e8) will decrease coverage by 0.07%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2827      +/-   ##
==========================================
- Coverage   99.18%   99.12%   -0.07%     
==========================================
  Files         115      115              
  Lines       17551    17551              
  Branches     3149     3149              
==========================================
- Hits        17408    17397      -11     
- Misses         99      109      +10     
- Partials       44       45       +1     

see 1 file with indirect coverage changes

@jakkdl
Copy link
Member Author

jakkdl commented Oct 23, 2023

wait hmm, the timeouts happening on Ubuntu (pypy-3.9) and Ubuntu (pypy-3.10) are some very different type of timeout than the CI being mad ... I think??
It's at least talking about some 1min timeout being triggered

@jakkdl
Copy link
Member Author

jakkdl commented Oct 23, 2023

Okay, actually digging into the traces I now realize this isn't about the CI killing at all - this seems to be Trio itself giving up in the main event loop. And it's not any particular test triggering it.

Manually going back and checking runs, the oldest I can find is https://github.com/python-trio/trio/actions/runs/6442036631/job/17492440655 .... in a PR that is modifying thread behaviour. So it's possible that #2392 has some subtle bugs, and should maybe be reverted for now?

@A5rocks
Copy link
Contributor

A5rocks commented Oct 24, 2023

TBH this PR isn't needed for an immediate issue anymore and does complicate CI a bit. Do you think it's still worth pursuing?

@jakkdl
Copy link
Member Author

jakkdl commented Oct 25, 2023

not especially. I do like the extra refactoring I did of pulling apart the previously huge and tricky-to-parse line though.

@jakkdl
Copy link
Member Author

jakkdl commented Oct 25, 2023

although: https://github.com/python-trio/trio/actions/runs/6641043117/job/18042685943?pr=2820

weirdly enough it displays as having passed despite being canceled???

@A5rocks
Copy link
Contributor

A5rocks commented Nov 18, 2023

Yeah I'm not sure what's up with that CI pass :(

I'm going to hope it's gone away. And we've cleaned up the line a bit more. Feel free to open a new PR (or reopen this and fix conflicts) if you still think it would benefit from more refactoring!

@A5rocks A5rocks closed this Nov 18, 2023
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.

3 participants