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

Remove redundant raw detach test #892

Merged
merged 1 commit into from
Mar 7, 2023

Conversation

st0012
Copy link
Member

@st0012 st0012 commented Feb 6, 2023

The test's subject is the disconnect request with restart: false, terminateDebuggee: <true|false> arguments, which has been covered by the
tests in disconnect_dap_test.rb. So this test case has become obsolete.

Background

I was investigating the master build failure with @andyw8. And we later found that this test case isn't designed to catch that error and actually is redundant. So by removing it we can also fix part of the master build failure.

If having a test to capture the stacktrace difference of the test failure is preferred, I'll be happy to open another PR for it. But IMO, it'd be testing Ruby's behaviour instead of testing this gem.

@st0012 st0012 changed the title Remove redundant and brittle raw detach test Remove redundant raw detach test Feb 6, 2023
@st0012 st0012 force-pushed the remove-unnecessary-raw-test branch 3 times, most recently from 3e4612f to 9fe6e67 Compare February 6, 2023 21:33
The test's subject is the `disconnect` request with `restart: false,
terminateDebuggee: <true|false>` arguments, which has been covered by the
tests in `disconnect_dap_test.rb`. So this test case has become obsolete.

Co-authored-by: Andy Waite <[email protected]>
@st0012 st0012 force-pushed the remove-unnecessary-raw-test branch from 9fe6e67 to 6e8f4d1 Compare February 27, 2023 20:07
@ko1 ko1 merged commit 13fc777 into ruby:master Mar 7, 2023
@ko1
Copy link
Collaborator

ko1 commented Mar 7, 2023

Thank you for investigation.

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.

2 participants