Uh oh!
There was an error while loading. Please reload this page.
Add assert_threads_result helper and threads test - #607
Conversation
abfc353 to
63e483fCompareUh oh!
There was an error while loading. Please reload this page.
ono-max
commented
Apr 2, 2022
Could you update CONTRIBUTING.md? |
st0012
commented
Apr 2, 2022
Sure. But speaking of documentation, do you think we can do that in Ruby comments? Like test-unit does. And in the contribution guideline we just link to the file. I think it fits the convention better and it's easier for writers of methods (like me) to remember too 😛 |
We can write how to use API in the comments and CONTRIBUTING.md. I think that writing the explanation in CONTRIBUTING.md is good because they are well organized. |
st0012
commented
Apr 2, 2022
I don't think having documentation in 2 places is a good idea. It's the same reason we only write command intro in |
6a0cc9c to
f66966fCompareono-max
commented
Apr 2, 2022
Well, I see. Or, we can create rdoc page from the comment? |
st0012
commented
Apr 2, 2022
Yes. In that case, we'll need to write the comments follow yard's specification. |
c54ba10 to
9b7333dComparest0012
commented
Apr 5, 2022
@ono-max I've updated the contribution guideline for now. can you check it? thx |
Uh oh!
There was an error while loading. Please reload this page.
ono-max
commented
Apr 6, 2022
Nice work! LGTM |
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Naoto Ono <onoto1998@gmail.com>
715079b to
6430f40CompareYou missed https://github.com/ruby/debug/pull/607/files#r843338555. Could you update it? |
Uh oh!
There was an error while loading. Please reload this page.
| expected_names.each do |expected| | ||
| thread_names.reject! do |name| | ||
| name.match?(expected) |
There was a problem hiding this comment.
We can use assert_match method in this case, could you tell me the reason why you implemented so?
There was a problem hiding this comment.
Why do you think we can use it here? Remember, the order of threads info is not guaranteed. So we don't know which one will match.
There was a problem hiding this comment.
No worries 😉
Do other changes look good to you?
6430f40 to
8b9a85eCompare| end | ||
| failure_msg = FailureMessage.new{create_protocol_message "result:\n#{JSON.pretty_generate res}.\nExpect all thread names to be matched. Unmatched threads:"} | ||
| assert_equal [], thread_names, failure_msg |


Some notes:
0and increment by1, as shown in the below example. So asserting ids is pointless imo.