Skip to content

Unify frame and locals collection implementation between ThreadClient and servers - #465

Merged
ko1 merged 1 commit into
ruby:masterfrom
st0012:share-methods-between-thread-client-and-server
Mar 20, 2022
Merged

Unify frame and locals collection implementation between ThreadClient and servers#465
ko1 merged 1 commit into
ruby:masterfrom
st0012:share-methods-between-thread-client-and-server

Conversation

@st0012

Copy link
Copy Markdown
Member
  1. Use get_frame to retrieve frames with the given frame index.
  2. Use collect_locals to retrieve the locals from the given frame.
    • This also fixes an issue that %self isn't displayed in DAP when stepping backward.

Thanks to the DAP tests written by @ono-max, I feel a lot comfortable doing such refactoring now 👍

@ono-max

ono-max commented Dec 30, 2021

Copy link
Copy Markdown
Member

Could you wait for refactoring CDP part until I'm done creating the test framework for CDP?
Thanks.

@st0012

Copy link
Copy Markdown
MemberAuthor

@ono-max I can drop the changes for CDP server if you concern about the test coverage.

@ono-max

Copy link
Copy Markdown
Member

That sounds good. Could you drop it? Thank you.

@st0012
st0012force-pushed the share-methods-between-thread-client-and-server branch from cd1ccb4 to dccd0c1CompareDecember 30, 2021 13:29
@st0012

Copy link
Copy Markdown
MemberAuthor

@ono-max done 👍

@ko1

ko1 commented Jan 4, 2022

Copy link
Copy Markdown
Collaborator

why dap tests are changed?

Comment threadtest/dap/step_back_test.rb
@ono-max

ono-max commented Jan 5, 2022

Copy link
Copy Markdown
Member

Could you rebase this PR so that we can confirm if your changes work in CI?

@st0012
st0012force-pushed the share-methods-between-thread-client-and-server branch from dccd0c1 to bcba6eaCompareJanuary 5, 2022 11:30
@st0012

Copy link
Copy Markdown
MemberAuthor

@ono-max rebased and it passed 👍

@st0012st0012 mentioned this pull request Jan 27, 2022
@ono-max

ono-max commented Feb 2, 2022

Copy link
Copy Markdown
Member

Sorry, if you're ok, could you use the test generator instead of modifying tests directly to rewrite tests?

@st0012
st0012force-pushed the share-methods-between-thread-client-and-server branch 2 times, most recently from 1572ccf to 5d4e33cCompareMarch 17, 2022 21:40
This change also fixes an issue that '%self' is not displayed when
stepping backward.
@st0012
st0012force-pushed the share-methods-between-thread-client-and-server branch from 5d4e33c to 34463a8CompareMarch 18, 2022 18:47
@st0012

Copy link
Copy Markdown
MemberAuthor

@ko1 I hope this can also be merged first if possible. I'll add more variables tests with #559

@ono-max

Copy link
Copy Markdown
Member

Did you fix tests with the test generator?

@st0012

Copy link
Copy Markdown
MemberAuthor

No. I plan to use the new assert_locals_result (or whatever you plan to call it) to test it.
The whole idea of adding unit-tests is that I don't need to rerun integration tests for changes like this.

@ko1

ko1 commented Mar 20, 2022

Copy link
Copy Markdown
Collaborator

Sorry for absent. I'll merge it and I revisit to consider about the name get_frame

@ko1
ko1 merged commit 3ef203d into ruby:masterMar 20, 2022
@st0012

Copy link
Copy Markdown
MemberAuthor

@ko1 thank you 🙏

@st0012
st0012 deleted the share-methods-between-thread-client-and-server branch March 20, 2022 22:23
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@st0012@ono-max@ko1