[stepping] Do not evaluate expressions on stepping#3530
Conversation
jasonLaster
left a comment
There was a problem hiding this comment.
Thanks!
couple of thoughts
| }); | ||
|
|
||
| case "COMMAND": | ||
| return { ...state, command: action.value.type }; |
There was a problem hiding this comment.
we should clear this field when it's done.
| dispatch(evaluateExpressions(null)); | ||
| if (!isStepping(getState())) { | ||
| dispatch(evaluateExpressions(null)); | ||
| } |
| return dispatch({ | ||
| type: "COMMAND", | ||
| value: undefined | ||
| value: { type } |
There was a problem hiding this comment.
lets add some jest unit tests to demonstrate that command is set and then cleared when the command is done.
I think we can do something like:
it("lala", async () => {
let resolve = null;
const slowClient = {
stepIn: () => new Promise(_resolve => {resolve = _resolve})
}
await actions.paused()
actions.stepIn()
expect(isStepping(state)).toBeTruthy()
resolve();
expect(isStepping(state)).toBeFalsey()
}
Codecov Report
@@ Coverage Diff @@
## master #3530 +/- ##
=========================================
+ Coverage 51.97% 52.4% +0.42%
=========================================
Files 110 110
Lines 4529 4536 +7
Branches 936 939 +3
=========================================
+ Hits 2354 2377 +23
+ Misses 2175 2159 -16
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #3530 +/- ##
==========================================
- Coverage 51.97% 51.94% -0.04%
==========================================
Files 110 110
Lines 4529 4534 +5
Branches 936 938 +2
==========================================
+ Hits 2354 2355 +1
- Misses 2175 2179 +4
Continue to review full report at Codecov.
|
|
|
||
| dispatch(evaluateExpressions(null)); | ||
| if (!isStepping(getState())) { | ||
| dispatch(evaluateExpressions(null)); |
There was a problem hiding this comment.
it would be nice to have a test here too:
+ describe("pause", () => {
+ it("should set and clear the command", async () => {
+ const client = { evaluate: jest.fn() };
+ const { dispatch, getState } = createStore(client);
+ dispatch(actions.stepIn());
+ await dispatch(actions.resumed());
+ expect(client.evaluate.calls.length).toEqual(0)
+ });
+ });
Associated Issue: #3493
Here's the Pull Request Doc
https://devtools-html.github.io/debugger.html/CONTRIBUTING.html#pull-requests
Summary of Changes
isSteppingto enable us know when we are steppingTest Plan