Skip to content

Protocol test framework improvements #654

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
Merged
merged 4 commits into from
Jun 24, 2022
Merged

Protocol test framework improvements #654

merged 4 commits into from
Jun 24, 2022

Conversation

st0012
Copy link
Member

@st0012 st0012 commented Jun 11, 2022 •
edited
Loading

I made these changes when debugging Ruby head's hanging issue and they did make the build failures easier to catch.

  1. Both debuggee connection and workflows should have timeout
  2. Rename WebSocketClient#cleanup to #close because that's all it does
  3. Only perform cleanup logic when the socket/threads are present, which may not be the case when the error happens during test setup.

@st0012 st0012 force-pushed the fix-protocol-test branch 23 times, most recently from a105e13 to dbfaee9 Compare June 13, 2022 21:22
@st0012 st0012 changed the title Add timeout to remote debuggee waiting logic Test improvements Jun 13, 2022
@st0012 st0012 force-pushed the fix-protocol-test branch from dbfaee9 to 0c2234b Compare June 13, 2022 22:03
@st0012 st0012 marked this pull request as ready for review June 13, 2022 23:09
st0012 added 4 commits June 24, 2022 21:04
@st0012
Add timeout to remote debuggee waiting logic
740ee6e
@st0012
Cleanup targets may be nil depends on the failure
3cb47b1
@st0012
Rename WebSocketClient#cleanup to #close because it's all it does
73d9a9e
@st0012
Add timeout to workflows
ab55aaf 
The default timeout is 6 hours, which is unnecessarily long. We should
stop tests if it doesn't finish in 10~15 mins to see what happens.
@st0012 st0012 force-pushed the fix-protocol-test branch from 0c2234b to ab55aaf Compare June 24, 2022 20:04
@st0012 st0012 changed the title Test improvements Protocol test framework improvements Jun 24, 2022
@st0012
Copy link
Member Author

st0012 commented Jun 24, 2022

@peterzhu2118 I could also use your help on this test improvement. You can run protocol tests with bundle exec rake test_protocol, which is not included in rake test.

peterzhu2118
peterzhu2118 approved these changes Jun 24, 2022
View reviewed changes
Copy link
Member

@peterzhu2118 peterzhu2118 left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

@peterzhu2118 peterzhu2118 merged commit 0e60ff3 into ruby:master Jun 24, 2022
@st0012 st0012 deleted the fix-protocol-test branch June 24, 2022 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peterzhu2118 peterzhu2118 peterzhu2118 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
@st0012 @peterzhu2118