Dev tools Network and Performance #7212

adiohana · 2019-05-16T09:19:37Z

Authors:
Adi Ohana
Shay Dratler
Behalf of Intuit

Summary:
Added Chrome DevTools Network and Performance API's
Added test scenarios that will run when running test_chrome command

Notes And Gaps:

Need to add support for List as Command input.
Add better error handling for DevTools operations
Add more test scenarios
Implement more DevTools methods according to https://chromedevtools.github.io/devtools-protocol/

By placing an X in the preceding checkbox, I verify that I have signed the Contributor License Agreement

Java chrome dev tools add performance api

devtools Network

added more parsers

* Complete Performance API + tests * Add BUCK + reveal * Adding test to ChromeTestSuite * Remove Dead Code - DevToolsApi * Make Testing available for all testing not only for junit

added tests changes

added parsers

…, responseReceived , responseReceived

Add Network methods and infra for it's tests

…vaChromeDevToolsAddPerformaceApi

network methods

shs96c

Wow. There's a lot here! Thank you! It's mostly great, too. Some general comments:

For enums, pick either SHOUTY or NonShouty. General Java style suggests the former, good taste and readability the latter.
The project code style is moving towards "one param per line once we need to line break"
Avoid using wrapper classes of primitive types where possible. People unbox with glee, and mayhem ensues.
Just because the CDP is daft, we don't need to be. If there are matching types in Selenium already, feel free to use them. java.time is a lovely package.

There's a few places where I've expanded on the reasoning behind the comments. I hope it proves helpful.

java/client/src/org/openqa/selenium/devtools/network/Network.java

shs96c · 2019-05-16T10:10:38Z

java/client/src/org/openqa/selenium/devtools/network/Network.java

+   * Returns all browser cookies. Depending on the backend support, will return detailed cookie information in the cookies field
+   * @return Array of cookies
+   */
+  public static Command<Set<Cookie>> getAllCookies() {


Selenium already has a org.openqa.selenium.Cookie. There will be lower friction with other parts of the Selenium APIs if we return extant Selenium types. I think it's fine to extend those types to be more meaningful if necessary.

@shs96c
In DevTools cookie the name of the fields are different (e.g expires VS expiry), this will cause the JSON deserializer to break. In addition, there are extra fields in DevTools Cookie.
What to do say about adding "asSeleniumCookie" to DevTools Cookie object. Then, the user can decide which Cookie he wants to use. Another option will be to always return the Selenium Cookie from DevTools command (and loose extra devtools cookie fields)

It's perfectly fine to have internal classes that never leak out of the package to make serialising/deserialising easy, but public APIs should be expressed in Selenium's own classes. Put another, having the "to/fromSeleniumCookie" method on CDP's Cookie class makes sense, but it should be package-level visible, rather than public.

added Cookies operations with Selenium Cookie.

the getCookies will be like:
devTools.send(getAllCookies()).asSeleniumCookies()
which will return List<org.openqa.selenium.Cookie>

the setCookie will just take Selenium Cookie.

java/client/test/org/openqa/selenium/chrome/DevToolsNetworkTest.java

java/client/test/org/openqa/selenium/chrome/DevToolsPerformanceTest.java

third_party/java/testng/BUCK

…class

adiohana · 2019-05-18T17:51:37Z

@shs96c

all review notes are handled.
1 open issue that we have is calling devtools operations inside a Consumer handler always timing out. This affecting all "Interception" operations where you must continue/block request in order to continue. I think we can handle it in the next phase.

let me know if we can merge this PR and let's discuss about more implementations.

shs96c

There's work to be done before we should ship to users --- lots of the data classes are mutable, there are a large number of primitive wrappers used in fields, and not all data classes have equals and hashCode implemented --- but this is a massive step forward.

shs96c · 2019-05-20T11:04:44Z

common/src/web/postForm.html

+<html>
+<body>
+
+<form method="post" action="resultPage.html">


It's probably fine now, but prefer creating new pages for tests: there are places where we count elements in tests because we didn't know better when we started writing these things.

shs96c · 2019-05-20T11:07:55Z

java/client/src/org/openqa/selenium/devtools/network/Network.java

+   * If a network fetch occurs as a result which encounters a redirect an additional Network.requestIntercepted event will be sent with the same InterceptionId.
+   * (EXPERIMENTAL)
+   *
+   * @param interceptionId        - Identifier for the intercepted request


Nit: this will end up with really weird looking javadocs -- the body of the description will begin - Identifier. There's also no need to worry about aligning the start of the descriptions.

We can leave as is and edit after merging unless you're feeling particularly energetic :)

shs96c · 2019-05-20T11:08:33Z

java/client/src/org/openqa/selenium/devtools/network/Network.java

+   * (EXPERIMENTAL)
+   *
+   * @param interceptionId        - Identifier for the intercepted request
+   * @param errorReason           (Optional) - If set this causes the request to fail with the given reason.


Nit: The type tells me that this parameter is Optional. There's no need to repeat that here. Same comment as above: we can fix after landing if necessary.

shs96c · 2019-05-20T11:29:32Z

java/client/src/org/openqa/selenium/devtools/network/Network.java

+   *
+   * @return Array of Cookies with a "asSeleniumCookies" method
+   */
+  public static Command<Cookies> getAllCookies() {


Set<Cookie> would map more nicely to java.

shs96c · 2019-05-20T11:30:10Z

java/client/src/org/openqa/selenium/devtools/network/Network.java

+   * @param urls (Optional) - The list of URLs for which applicable cookies will be fetched
+   * @return Array of cookies
+   */
+  public static Command<Cookies> getCookies(Optional<List<String>> urls) {


And this should also be Set<Cookie>

shs96c · 2019-05-20T12:15:56Z

java/client/src/org/openqa/selenium/devtools/network/model/SignedExchangeReceived.java

+          while (input.hasNext()) {
+            switch (input.nextName()) {
+              case "outerResponse":
+                outerResponse = Response.parseResponse(input);


If Response has a private Response fromJson(JsonInput) method json.read(Response.class) will work.

shs96c · 2019-05-20T12:16:48Z

java/client/src/org/openqa/selenium/devtools/network/model/SignedExchangeReceived.java

+                securityDetails = SecurityDetails.parseSecurityDetails(input);
+                break;
+              case "errors":
+                input.beginArray();


Similarly: json.read(new TypeToken<List<SignedExchangeError>>(){}.getType())

shs96c · 2019-05-20T12:19:02Z

java/client/src/org/openqa/selenium/json/JsonInputConverter.java

+    return (null != number) ? number.intValue() : null;
+  }
+
+  public static Map<String,Object> extractMap(JsonInput input){


This can be replaced with input.read(Json.MAP_TYPE)

shs96c · 2019-05-20T12:19:51Z

java/client/src/org/openqa/selenium/json/JsonInputConverter.java

+
+public class JsonInputConverter {
+  public static Double extractDouble(JsonInput input){
+    Number number = input.nextNumber();


This is equivalent to input.read(Double.class)

shs96c · 2019-05-20T12:20:20Z

java/client/src/org/openqa/selenium/json/JsonInputConverter.java

+    return (null != number) ? number.doubleValue() : null;
+  }
+
+  public static Integer extractInt(JsonInput input){


This is equivalent to input.read(Integer.class)

danhumphrey · 2019-12-13T02:26:24Z

@adiohana I know this is merged, but you mention the ability to manipulate responses, however, I can't see how.

In puppeteer this would be done with page.setRequestInterception(true), then using something like:

page.on("request", request => {
  ...
  request.respond({
    status: 400
    ...
  })
}

I've looked at Network.requestIntercepted() , Network.responseReceived() etc. I've also looked at Fetch.requestPaused, but can't see how to mock the actual response status and/or body.

Could you please point me in the right direction?

Thanks!

Shay Dratler and others added 19 commits May 15, 2019 10:52

Current works

adab44b

Merge pull request #1 from adiohana/javaChromeDevToolsAddPerformaceApi

75c7f87

Java chrome dev tools add performance api

devtools Network

55e87e0

Merge pull request #2 from adiohana/dev_tools_network

51dd032

devtools Network

added more parsers

1438d18

Merge pull request #3 from adiohana/dev_tools_network

50fe76d

added more parsers

* Network add Cookies

5d402d2

* Complete Performance API + tests * Add BUCK + reveal * Adding test to ChromeTestSuite * Remove Dead Code - DevToolsApi * Make Testing available for all testing not only for junit

merge commit

ac2dea4

added tests changes

503d0d5

Merge pull request #5 from adiohana/dev_tools_network

3e21730

added tests changes

added parsers

3eb305a

Merge pull request #6 from adiohana/dev_tools_network

6c79b73

added parsers

fix merge conflicts

bf40dfb

Add Methods webSocketClosed , webSocketCreated , webSocketFrameError …

ac821ec

…, responseReceived , responseReceived

Merge pull request #7 from adiohana/javaChromeDevToolsAddPerformaceApi

8d3fef7

Add Network methods and infra for it's tests

Network methods completed

2a3dff7

Merge branch 'master' of https://github.com/adiohana/selenium into ja…

f9a34d6

…vaChromeDevToolsAddPerformaceApi

Merge pull request #8 from adiohana/javaChromeDevToolsAddPerformaceApi

93dd215

network methods

finish up

1defb64

adiohana mentioned this pull request May 16, 2019

Java implementation of DevTools API #6667

Closed

shs96c requested changes May 16, 2019

View reviewed changes

aohana added 2 commits May 16, 2019 17:58

fixes according to comments

eeb5185

fixes according to comments #2 - null validations

e6622a5

p0deje added the C-java label May 17, 2019

aohana added 5 commits May 17, 2019 10:05

null validations in all model classes. use Instant for all timestamps.

1c65237

added test cases

2019545

use AppServer in devtools test cases. created abstract DevTools base …

cfe501c

…class

cookie operations will be with Selenium Cookie

e581e78

added test coverage for Network. changed unsupported methods to private

b810020

added test coverage for Network. support Command List as input

4f76757

adiohana mentioned this pull request May 20, 2019

What Capabilities of chrome driver is used to show latency in the network or set offline mode? #7220

Closed

shs96c approved these changes May 20, 2019

View reviewed changes

added copyrights

38824dc

shs96c merged commit 6d4ebac into SeleniumHQ:master May 20, 2019

admizh mentioned this pull request May 25, 2019

Add ability to collect http(s) requests to HAR file without proxy (browserMob) selenide/selenide#190

Closed

dratler deleted the dev_tools_apis branch May 29, 2019 18:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dev tools Network and Performance #7212

Dev tools Network and Performance #7212

adiohana commented May 16, 2019 •

edited

Loading

shs96c left a comment

shs96c May 16, 2019

adiohana May 16, 2019

shs96c May 17, 2019

adiohana May 18, 2019

adiohana commented May 18, 2019 •

edited

Loading

shs96c left a comment

shs96c May 20, 2019

shs96c May 20, 2019

shs96c May 20, 2019

shs96c May 20, 2019

shs96c May 20, 2019

shs96c May 20, 2019

shs96c May 20, 2019

shs96c May 20, 2019

shs96c May 20, 2019

shs96c May 20, 2019

danhumphrey commented Dec 13, 2019 •

edited

Loading

Dev tools Network and Performance #7212

Dev tools Network and Performance #7212

Conversation

adiohana commented May 16, 2019 • edited Loading

shs96c left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adiohana commented May 18, 2019 • edited Loading

shs96c left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danhumphrey commented Dec 13, 2019 • edited Loading

adiohana commented May 16, 2019 •

edited

Loading

adiohana commented May 18, 2019 •

edited

Loading

danhumphrey commented Dec 13, 2019 •

edited

Loading