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

Avoid calling @sock.close for CDP tests #565

Closed
wants to merge 1 commit into from

Conversation

st0012
Copy link
Member

@st0012 st0012 commented Mar 14, 2022

@sock is only defined when running DAP tests:

def attach_to_dap_server
@sock = Socket.unix @remote_info.sock_path

Therefore, @sock will be nil when finishing CDP tests and cause NoMethodError.

And we haven't seen errors because DAP tests are always ran before CDP's. So the @sock variable will be left assigned. If we use dap: false in any protocol test, we can see the error:

Error: test_detach_reattach_to_rdbg(DEBUGGER__::DetachTest):
  NoMethodError: undefined method `close' for nil:NilClass

        @sock.close
             ^^^^^^
/Users/st0012/projects/debug/test/support/protocol_utils.rb:271:in `ensure in execute_cdp_scenario'
/Users/st0012/projects/debug/test/support/protocol_utils.rb:274:in `execute_cdp_scenario'
/Users/st0012/projects/debug/test/support/protocol_utils.rb:47:in `run_protocol_scenario'
test/protocol/detach_test.rb:23:in `test_detach_reattach_to_rdbg'
     20:     RUBY
     21:
     22:     def test_detach_reattach_to_rdbg
  => 23:       run_protocol_scenario PROGRAM, dap: false do
     24:         assert_reattach
     25:         req_terminate_debuggee
     26:       end

@st0012 st0012 force-pushed the fix-protocol-tests branch from 049db87 to 8a76362 Compare March 14, 2022 00:39
@ono-max
Copy link
Member

ono-max commented Mar 14, 2022

Thanks. I already fixed in #499.

@st0012
Copy link
Member Author

st0012 commented Mar 14, 2022

@ono-max Ok, I'll close this. But do you think you can open a separate PR for the fix? Currently it's not possible to run only CDP tests and it bothers me when testing around the framework.

@st0012 st0012 closed this Mar 14, 2022
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