Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(106)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2890463004: [inspector] Refactor inspector test (Closed)

Created:
3 years, 7 months ago by dgozman
Modified:
3 years, 7 months ago
Reviewers:
kozy
CC:
v8-reviews_googlegroups.com, devtools-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
v8
Visibility:
Public.

Description

[inspector] Refactor inspector test - moved all extensions to inspector_test.cc; - properly supported multiple context groups and sessions; - better isolation between components; - better infrastructure in protocol-test. BUG=chromium:590878 Review-Url: https://codereview.chromium.org/2890463004 Cr-Commit-Position: refs/heads/master@{#45409} Committed: https://chromium.googlesource.com/v8/v8/+/55849b167c16bb7f87b5c747b02d27f531bc4086

Patch Set 1 #

Patch Set 2 : works #

Total comments: 6

Patch Set 3 : comments addressed #

Patch Set 4 : fix crash #

Patch Set 5 : better crash fix #

Unified diffs Side-by-side diffs Delta from patch set Stats (+775 lines, -689 lines) Patch
M test/inspector/console/destroy-context-during-log.js View 2 chunks +4 lines, -4 lines 0 comments Download
M test/inspector/debugger/destory-in-break-program.js View 1 chunk +1 line, -1 line 0 comments Download
M test/inspector/debugger/framework-break.js View 1 chunk +3 lines, -3 lines 0 comments Download
M test/inspector/debugger/framework-precise-ranges.js View 1 chunk +1 line, -1 line 0 comments Download
M test/inspector/debugger/framework-stepping.js View 3 chunks +3 lines, -3 lines 0 comments Download
M test/inspector/debugger/get-possible-breakpoints.js View 1 chunk +1 line, -1 line 0 comments Download
M test/inspector/debugger/inspector-break-api.js View 1 chunk +4 lines, -4 lines 0 comments Download
M test/inspector/debugger/pause.js View 2 chunks +26 lines, -22 lines 0 comments Download
M test/inspector/debugger/step-over-another-context-group.js View 1 chunk +11 lines, -9 lines 0 comments Download
M test/inspector/debugger/stepping-ignores-injected-script.js View 1 chunk +1 line, -1 line 0 comments Download
M test/inspector/debugger/stepping-with-exposed-injected-script.js View 1 chunk +1 line, -1 line 0 comments Download
M test/inspector/debugger/wasm-scripts.js View 1 chunk +1 line, -2 lines 0 comments Download
M test/inspector/inspector-impl.h View 1 2 3 4 4 chunks +30 lines, -54 lines 0 comments Download
M test/inspector/inspector-impl.cc View 1 2 3 4 6 chunks +77 lines, -222 lines 0 comments Download
M test/inspector/inspector-test.cc View 1 2 3 18 chunks +367 lines, -183 lines 0 comments Download
M test/inspector/isolate-data.h View 1 3 chunks +6 lines, -1 line 0 comments Download
M test/inspector/isolate-data.cc View 1 6 chunks +17 lines, -1 line 0 comments Download
M test/inspector/json-parse.js View 1 chunk +2 lines, -2 lines 0 comments Download
M test/inspector/protocol-test.js View 1 2 8 chunks +138 lines, -104 lines 0 comments Download
M test/inspector/runtime/create-context.js View 1 2 chunks +23 lines, -16 lines 0 comments Download
M test/inspector/runtime/runtime-restore.js View 1 2 chunks +4 lines, -4 lines 0 comments Download
M test/inspector/task-runner.h View 1 2 4 chunks +18 lines, -19 lines 0 comments Download
M test/inspector/task-runner.cc View 1 2 7 chunks +36 lines, -31 lines 0 comments Download

Depends on Patchset:

Issue 2887013002 Patch 1

Messages

Total messages: 29 (20 generated)
dgozman
Could you please take a look?
3 years, 7 months ago (2017-05-18 17:59:17 UTC) #8
dgozman
Could you please take a look?
3 years, 7 months ago (2017-05-18 17:59:21 UTC) #9
kozy
lgtm! https://codereview.chromium.org/2890463004/diff/20001/test/inspector/protocol-test.js File test/inspector/protocol-test.js (right): https://codereview.chromium.org/2890463004/diff/20001/test/inspector/protocol-test.js#newcode360 test/inspector/protocol-test.js:360: var Protocol = InspectorTest.session.Protocol; this.Protocol = ... https://codereview.chromium.org/2890463004/diff/20001/test/inspector/task-runner.cc ...
3 years, 7 months ago (2017-05-18 18:35:28 UTC) #13
dgozman
https://codereview.chromium.org/2890463004/diff/20001/test/inspector/protocol-test.js File test/inspector/protocol-test.js (right): https://codereview.chromium.org/2890463004/diff/20001/test/inspector/protocol-test.js#newcode360 test/inspector/protocol-test.js:360: var Protocol = InspectorTest.session.Protocol; On 2017/05/18 18:35:28, kozy wrote: ...
3 years, 7 months ago (2017-05-18 19:03:21 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2890463004/40001
3 years, 7 months ago (2017-05-18 19:05:19 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2890463004/60001
3 years, 7 months ago (2017-05-18 19:59:55 UTC) #21
commit-bot: I haz the power
Try jobs failed on following builders: v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/22729) v8_mac_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, ...
3 years, 7 months ago (2017-05-18 20:47:23 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2890463004/80001
3 years, 7 months ago (2017-05-18 22:32:50 UTC) #26
commit-bot: I haz the power
3 years, 7 months ago (2017-05-18 23:11:30 UTC) #29
Message was sent while issue was closed. 
Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/v8/v8/+/55849b167c16bb7f87b5c747b02d27f531b...

Powered by Google App Engine
This is Rietveld 408576698