[java] implement performance support in session #270

titusfortner · 2021-07-22T01:05:03Z

Three options here.

The one I did for Accessibility was like:

PerformanceResults results = session.getPerformanceResults();

The one that is probably "most Java" is:

PerformanceResults results = new Performance(session).getResults();

The one I implemented here is:

PerformanceResults results = session.performance().getResults();

Thoughts/Preferences?

nadvolod

I like this approach
PerformanceResults results = session.performance().getResults();
I think that the API usage makes sense and I also like having the performance stored in a separate object Performance.

java/main/src/main/java/com/saucelabs/saucebindings/performance/Performance.java

nadvolod · 2021-07-22T14:13:11Z

java/main/src/test/java/com/saucelabs/saucebindings/budget.yml

@@ -0,0 +1,6 @@
+"https://www.saucedemo.com":


@titusfortner can you please explain what this is and it's intended usage?

So, for JS they use JSON; I wanted to implement something that uses a Map, so you can convert either yaml or json into the map it will work. I didn't want to use JSON because I don't want to deal with Jackson library, yet. :) More info: https://docs.saucelabs.com/performance/transitions#defining-a-performance-budget

@titusfortner I worry that this feature is unnecessarily making the code complicated and overengineered. From my experience, customers aren't advanced enough to handle such a use case (or even me 😂). Instead they are only interested in 3 use cases. Specifically they like multiPagePerformanceExample() and the simplePerformanceTest()

Yeah, We have docs for it with code examples, so I thought we should provide support for it. I'm not sure how much it would be used. I had some thoughts on expanding it later, but definitely not the thing that should be holding us up.

This is the kind of feature I wouldn't make an issue out of in the Sauce Bindings Documentation, but would have example code in the Sauce Docs with it for people who are interested in playing with it. As such, the confusion factor of the average user shouldn't really matter. It's the same kind of thing with the mergeCapabilities() in SauceOptions.

java/main/src/test/java/com/saucelabs/saucebindings/integration/PerformanceTest.java

nadvolod · 2021-07-22T14:18:06Z

java/main/src/test/java/com/saucelabs/saucebindings/integration/PerformanceTest.java

+    public void performanceDetails() {
+        driver.get("https://www.saucedemo.com");
+
+        PerformanceMetrics performanceMetrics = session.performance().getMetrics();


This confuses me. I don't understand the difference between a PerformanceMetrics and PerformanceResults

Maybe we rename PerformanceMetrics to PerformanceLog.

PerformanceResults uses Sauce Labs magic AI values. You can pass in specific metrics you care about and Sauce will pass/fail it based on whether we think it is in range.

If you want to assert on anything based on a hard coded value instead of our magic, you need to first get the log.

I mean, maybe this should be:

PerformanceLogs performanceLogs = session.logs().getPerformance();

Makes sense. The API call would be what you said or session.performance().getLogs()

The other part of this is that it isn't all the logs, just the logs for the last page transition. session.logs() should be getting all the logs for something, so maybe session.performance().getLastLog()?

@titusfortner so if it's just for the last page transition then maybe?
session.performance().getLastPageTransitionLog()

java/main/src/test/java/com/saucelabs/saucebindings/integration/PerformanceTest.java

nadvolod · 2021-07-23T13:19:52Z

java/main/src/test/java/com/saucelabs/saucebindings/integration/PerformanceTest.java

+    public void performanceMetricResults() {
+        driver.get("https://www.saucedemo.com");
+
+        PerformanceResults performanceResults = session.performance().getResults("firstInteractive");


@titusfortner should we move the parameter values into an enum? They've remained the same forever and it should improve usability

Yeah, so long as we have getRawResults() I don't think there's a problem doing enums for this.

titusfortner · 2022-02-17T00:46:15Z

java/main/src/main/java/com/saucelabs/saucebindings/SauceSession.java

@@ -75,6 +77,13 @@ public Results getAccessibilityResults(AxeBuilder builder) {
        return builder.analyze(driver);
    }

+    public Performance performance() {
+        if (!sauceOptions.sauce().getCapturePerformance()) {


what does this do for non-Chrome which doesn't have this method?

titusfortner · 2022-02-17T00:50:41Z

java/main/src/main/java/com/saucelabs/saucebindings/performance/Performance.java

+     *
+     * @param metrics A List of values for which performance metrics to evaluate
+     * @return PerformanceResults is a wrapper of the HashMap results to make it easier to work with them
+     * @see <a href="https://docs.saucelabs.com/performance/transitions#implementing-the-performance-command-assertion">Implementing the Performance Command Assertion</a>


these links are wrong for anchors, they need to be /# without index.html

titusfortner · 2022-02-17T00:55:14Z

java/main/src/main/java/com/saucelabs/saucebindings/performance/Performance.java

+    /**
+     * This measures the performance output against a baseline of previously accepted performance values
+     *
+     * @return PerformanceResults is a wrapper of the HashMap results to make it easier to work with them


specify this is passed, reason, and details

titusfortner · 2022-03-31T14:50:17Z

java/main/src/main/java/com/saucelabs/saucebindings/performance/Performance.java

+     * @return PerformanceResults is a wrapper of the HashMap results to make it easier to work with them
+     * @see <a href="https://docs.saucelabs.com/performance/transitions#implementing-the-performance-command-assertion">Implementing the Performance Command Assertion</a>
+     */
+    public PerformanceResults getResults() {


probably needs to be getAllResults() to better differentiate from other method

[java] implement performance support in session

e53e0fc

titusfortner requested review from nadvolod, c3ccl3ston and eyaly July 22, 2021 01:05

titusfortner added 4 commits July 21, 2021 20:32

[java] allow performance results to be limited by specific metrics

fe2793d

[java] add jankiness support to performance

4991d8a

[java] implement support for performance budget Map

6fec307

[java] add javadocs for performance

436fdfa

titusfortner force-pushed the performance_java branch from f85c1ad to 436fdfa Compare July 22, 2021 05:42

nadvolod reviewed Jul 22, 2021

View reviewed changes

nadvolod linked an issue Jul 22, 2021 that may be closed by this pull request

Add Sauce performance enablement and disablement #247

Open

nadvolod reviewed Jul 23, 2021

View reviewed changes

titusfortner commented Feb 17, 2022

View reviewed changes

titusfortner commented Mar 31, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[java] implement performance support in session #270

[java] implement performance support in session #270

titusfortner commented Jul 22, 2021

nadvolod left a comment

nadvolod Jul 22, 2021

titusfortner Jul 22, 2021

nadvolod Jul 23, 2021

titusfortner Jul 23, 2021

nadvolod Jul 22, 2021

titusfortner Jul 22, 2021

nadvolod Jul 26, 2021

titusfortner Jul 26, 2021

nadvolod Jul 29, 2021

nadvolod Jul 23, 2021

titusfortner Jul 26, 2021

titusfortner Feb 17, 2022

titusfortner Feb 17, 2022

titusfortner Feb 17, 2022

titusfortner Mar 31, 2022

[java] implement performance support in session #270

Are you sure you want to change the base?

[java] implement performance support in session #270

Conversation

titusfortner commented Jul 22, 2021

nadvolod 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

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