[wasm][debugger] Removed duplicated testcases as a follow-up for #70560 #71827
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Tagging subscribers to 'arch-wasm': @lewing Issue DetailsTheories added in #70560 contained duplicated testcases with the existing ones. The previous PR aimed at correcting the logic + adding new tests. This one cleans up the duplicates and renames some entities to make the tests clearer.
|
radical
reviewed
Jul 18, 2022
src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs
Outdated
Show resolved
Hide resolved
radical
reviewed
Jul 18, 2022
src/mono/wasm/debugger/tests/debugger-test/debugger-evaluate-test.cs
Outdated
Show resolved
Hide resolved
radical
reviewed
Jul 18, 2022
radical
reviewed
Jul 18, 2022
Member
radical
left a comment
radical
left a comment
There was a problem hiding this comment.
Other than the feedback, the changes look good.
lewing
approved these changes
Jul 19, 2022
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Theories added in #70560 contained duplicated testcases with the existing ones. The previous PR aimed at correcting the logic + adding new tests. This one cleans up the duplicates and renames some entities to make the tests clearer.
Removals and reasons:
EvaluateAsyncMethodsis removed because inside there was a mess - inside we had astaticClassthat was in fact, an instance class and the method did not test anything async. It was replaced in the previous PR with:EvaluateNonStaticClassWithStaticFieldswith 4 different methods out of which we check the OnEval reply: InstanceMethod (Run), InstanceAsyncMethod (RunAsync), StaticMethod (RunStatic), StaticAsyncMethod (RunStaticAsync)and
EvaluateStaticFieldsInStaticClasswith similar set of methods: StaticMethod (Run) and StaticAsyncMethod (RunAsync).EvaluateAsyncMethodsgot removed, it means:test:
EvaluateStaticClassFromStaticMethod(got replaced withEvaluateStaticFields)test data:
yield return new object[]....(got replaced withInlineDataforEvaluateStaticFields).