Bug 238806 – [JSC] Report test flakiness to resultsdb

• Component: New Bugs
• Status: RESOLVED FIXED

[Angelos Oikonomopoulos]

[JSC] Report test flakiness to resultsdb

[Angelos Oikonomopoulos]

Created attachment 456704 [details]
Patch

[Angelos Oikonomopoulos]

Note that this depends on at least https://bugs.webkit.org/show_bug.cgi?id=238807 (so that the upload will not fail). Something like https://bugs.webkit.org/show_bug.cgi?id=238809 is needed to visualize the submitted information.

[Angelos Oikonomopoulos]

Created attachment 456912 [details]
Patch

[Angelos Oikonomopoulos]

(In reply to Angelos Oikonomopoulos from comment #2)
> Note that this depends on at least
> https://bugs.webkit.org/show_bug.cgi?id=238807 (so that the upload will not
> fail). Something like https://bugs.webkit.org/show_bug.cgi?id=238809 is
> needed to visualize the submitted information.

The new version of the patch no longer uses a nested dict, so it no longer depends on https://bugs.webkit.org/show_bug.cgi?id=238807. AFAIK it doesn't /depend/ on https://bugs.webkit.org/show_bug.cgi?id=238809 and my testing seems to confirm that.

[Jonathan Bedard]

Comment on attachment 456912 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=456912&action=review

> Tools/Scripts/run-javascriptcore-tests:984
> +            $reportData{$testName}{"flakiness_num_passes"} = int($numPasses);

I would like an example of a an upload json with this data to throw against our staging instance to make sure nothing break.

[Angelos Oikonomopoulos]

Created attachment 457239 [details]
Report with some passes, some failures, some flakiness

Generated by running:

$ ./Tools/Scripts/run-javascriptcore-tests --no-build --jsc-only  --report=http://someLocalAddress --treat-failing-as-flaky=0.5,100,50

with this local patch (on top of the actual patch in this bug):

diff --git a/JSTests/stress/array-slice-osr-exit.js b/JSTests/stress/array-slice-osr-exit.js
index 980feace0623..eb01e8cf0251 100644
--- a/JSTests/stress/array-slice-osr-exit.js
+++ b/JSTests/stress/array-slice-osr-exit.js
@@ -71,4 +71,8 @@ function runTests() {
     }
 }
 
+if (Math.random() > 0.3) {
+       throw new Error("Failed");
+}
+
 runTests();
diff --git a/Tools/Scripts/run-javascriptcore-tests b/Tools/Scripts/run-javascriptcore-tests
index c994b42163cb..f5fccb50f24a 100755
--- a/Tools/Scripts/run-javascriptcore-tests
+++ b/Tools/Scripts/run-javascriptcore-tests
@@ -1095,10 +1095,14 @@ sub uploadResults
     }
 
     print "Uploading results to $report\n";
-    open(HANDLE, "|-", "curl -X POST $report/api/upload -H 'Content-Type: application/json' -f -d '\@-'") or die "Failed to open curl";
 
     # Json conforming to https://results.webkit.org/documentation#API-Uploads.
     my $encodedUpload = encode_json(\%upload);
+
+    open(DATA, ">report.json");
+    print DATA "$encodedUpload\n\0";
+    #return 0;
+    open(HANDLE, "|-", "curl -X POST $report/api/upload -H 'Content-Type: application/json' -f -d '\@-'") or die "Failed to open curl";
     print HANDLE "$encodedUpload\n\0";
     my $success = close HANDLE;

[Angelos Oikonomopoulos]

(In reply to Angelos Oikonomopoulos from comment #6)
> Created attachment 457239 [details]
> Report with some passes, some failures, some flakiness
> 
> Generated by running:
> 
> $ ./Tools/Scripts/run-javascriptcore-tests --no-build --jsc-only 
> --report=http://someLocalAddress --treat-failing-as-flaky=0.5,100,50

Oops, this also includes --filter=array-slice-osr-exit of course, copied the wrong line from my shell history.

[Angelos Oikonomopoulos]

Created attachment 457248 [details]
Full report with some artificially introduced flakiness

[Angelos Oikonomopoulos]

(In reply to Jonathan Bedard from comment #5)
> Comment on attachment 456912 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=456912&action=review
> 
> > Tools/Scripts/run-javascriptcore-tests:984
> > +            $reportData{$testName}{"flakiness_num_passes"} = int($numPasses);
> 
> I would like an example of a an upload json with this data to throw against
> our staging instance to make sure nothing break.

Right. Added two json files, a "minimal" one (with --filter) and a "full" one (without --filter). Both have some flaky tests. I can submit them to a local resultsdb instance using

cat report.json | curl -X POST http://address:5000/api/upload -H 'Content-Type: application/json' -f -d '@-'

Let me know if there's anything else I can do that would help with independent testing.

[Jonathan Bedard]

Comment on attachment 456912 [details]
Patch

I'm r+ing but not cq+ing because the report.json is valid, but minimal-report.json is not. I'm assuming minimal-report.json is basically a failed attempt at stripping out results not needed to illustrate the feature, but I'd like Angelos to verify that assumption before landing.

[Angelos Oikonomopoulos]

(In reply to Jonathan Bedard from comment #10)
> Comment on attachment 456912 [details]
> Patch
> 
> I'm r+ing but not cq+ing because the report.json is valid, but
> minimal-report.json is not. I'm assuming minimal-report.json is basically a
> failed attempt at stripping out results not needed to illustrate the
> feature, but I'd like Angelos to verify that assumption before landing.

This is strange; minimal-report.json /is/ machine generated. The main difference is that I also passed --filter=array-slice-osr-exit to run-javascriptcore-tests to only run the testcase I'd introduced some flakiness to.

Stranger still, I can't reproduce an issue with it. I've downloaded the file from bugzilla to make sure I'm testing the same thing (this is with async_processing=False):

$ curl https://bug-238806-attachments.webkit.org/attachment.cgi?id=457239 > report-minimal.json
$ cat report-minimal.json | curl -X POST http://127.0.0.1:5000/api/upload -H 'Content-Type: application/json' -f -d '@-'
{"processing":{"ci-urls":{"status":"ok"},"commit-identifiers":{"status":"ok"},"suite-results":{"status":"ok"},"test-failures":{"status":"ok"},"test-results":{"status":"ok"}},"status":"ok"}
$ sha256sum report-minimal.json 
5f570569f2430c027305d63629e110172dc96bb4f5ec951e8fb3394d455da78e  report-minimal.json

I've tried both with --local-redis --local-cassandra and --mock-redis --mock-cassandra and the result is the same: a 200 in the resultsdb stderr and subsequent API calls return the data.

Note that, in my setup, the minimal report works with async_processing=True too, as long as I use --local-redis (seems the async processing job is not picked up when using --mock-redis).

Assuming minimal-report.json has the same checksum for you, could you let me know what commit the staging resultsdb instance is running on? Maybe I'll have better luck reproducing the issue then.

[Radar WebKit Bug Importer]

<rdar://problem/91629460>

[Jonathan Bedard]

(In reply to Angelos Oikonomopoulos from comment #11)
> ...

Seems like I fat-fingered something with `curl` yesterday...tried it this morning (with an unmodified file) and things work fine if I disable our API key. As a small note, results.webkit.org (and our staging instance) will reject uploads without the correct API key. This is passed to scripts via an environment variable and been working for a few years at this point. I was modifying these files slightly to add the API key, that's likely to blame for my mistake.

[Angelos Oikonomopoulos]

Glad we found a plausible explanation for this!

[EWS]

Committed r292775 (249559@main): <https://commits.webkit.org/249559@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 456912 [details].