ruby / debug

Debugging functionality for Ruby
BSD 2-Clause "Simplified" License
1.1k stars 125 forks source link

Fix test framework for #688 #689

Open ono-max opened 1 year ago

ono-max commented 1 year ago

https://github.com/ruby/debug/pull/688

ono-max commented 1 year ago

I'll fix it later.

st0012 commented 1 year ago

For the DAP test, the fix is the same as: https://github.com/ruby/debug/pull/640/files#diff-edcbc39076c34506ea43bae159c4b26104156861edc7b84224e85cc4e980d355

ono-max commented 1 year ago

Oh, I see.

Could you create the PR instead of me?

st0012 commented 1 year ago

@ono-max Do you agree to drop the detach raw test? I think in this case it's unnecessary because the request test cover the exact same behavior. The rest of requests have also been tested in other parts of request tests.

ono-max commented 1 year ago

Thank you for asking me. I think that more test cases are better than less test cases, though?

Although I wrote the raw test in https://github.com/ruby/debug/pull/705, I'll close it if you disagree with it.

st0012 commented 1 year ago

I think that more test cases are better than less test cases, though?

That's true but we also need to consider the maintenance cost. If you look at #705, you can see that the test content may have been outdated for a while. And that makes me think:

I of course appreciate your work on #705 so I don't think we should close it. But if the raw tests in the current form are worth maintaining is debatable imo.

st0012 commented 1 year ago

How about this: if you can list one or more scenarios that a raw test case covers that its request counterpart doesn't, we keep it. In addition to that, we should also document it done in the file so it'll help us maintain it. If not, we remove it.