Bug 208420 - Web Inspector: introduce a proxy-based protocol test harness
Summary: Web Inspector: introduce a proxy-based protocol test harness
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Pavel Feldman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-02-29 16:41 PST by Pavel Feldman
Modified: 2020-05-08 10:46 PDT (History)
2 users (show)

See Also:


Attachments
Patch (6.48 KB, patch)
2020-02-29 16:45 PST, Pavel Feldman
hi: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pavel Feldman 2020-02-29 16:41:40 PST
Here is how it looks:

    const protocol = InspectorProtocol.protocol;

    // Setup interception.
    await Promise.all([protocol.Runtime.enable(), protocol.Network.enable()]);
    await protocol.Network.setInterceptionEnabled({ enabled: true });

    // Get URL to override and override it.
    const { result } = await protocol.Runtime.evaluate({ expression: "document.URL" });
    const url = result.value.substring(0, result.value.lastIndexOf("/")) + "/resources/data.json";
    await protocol.Network.addInterception({ url, stage: "response" });

    // Fetch URL and intercept it.
    protocol.Runtime.evaluate({ expression: "fetch('resources/data.json').then(r => r.text())"});
    const event = await protocol.Network.onceResponseIntercepted();
    ProtocolTest.pass("Intercepted response.");

    // // Let the load continue.
    await Promise.all([
        protocol.Network.onceResponseReceived(),
        protocol.Network.interceptContinue({ requestId: event.params.requestId })
    ]);
    ProtocolTest.pass("Response received.");

    await protocol.Network.removeInterception({ url, stage: "response" });

WDYT?
Comment 1 Pavel Feldman 2020-02-29 16:45:41 PST
Created attachment 392080 [details]
Patch
Comment 2 Devin Rousso 2020-03-02 11:33:50 PST
Comment on attachment 392080 [details]
Patch

r-, I _really_ don't like this approach.  It now means that code is no longer searchable.  Most (if not all) IDEs will be unable to "Jump to Definition".  Why is this needed?  Personally, I don't think the code is any more readable than it was before (and I didn't have a problem before).
Comment 3 Pavel Feldman 2020-03-02 16:22:52 PST
No worries.

We used to have what you have and then added this proxy mode and that boosted the number of protocol tests that we were able to produce. Basically we went from 10s to 100s. Protocol tests are cheap and fast, so all the means that improve their readability were bug wins. Third-parties were also able to finally read our tests as well.

While I obviously don't insist, I find it much more readable and I would encourage you do to a little poll among your colleagues - maybe someone likes it like myself...

>> It now means that code is no longer searchable.

There was no way to jump to the definition before either, so it is not that we are losing much here.