Bug 238806 - [JSC] Report test flakiness to resultsdb
Summary: [JSC] Report test flakiness to resultsdb
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Angelos Oikonomopoulos
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-04-05 09:03 PDT by Angelos Oikonomopoulos
Modified: 2022-04-12 11:21 PDT (History)
3 users (show)

See Also:


Attachments
Patch (17.41 KB, patch)
2022-04-05 09:04 PDT, Angelos Oikonomopoulos
no flags Details | Formatted Diff | Diff
Patch (17.48 KB, patch)
2022-04-07 05:32 PDT, Angelos Oikonomopoulos
no flags Details | Formatted Diff | Diff
Report with some passes, some failures, some flakiness (3.74 KB, application/json)
2022-04-11 02:22 PDT, Angelos Oikonomopoulos
no flags Details
Full report with some artificially introduced flakiness (3.69 MB, application/json)
2022-04-11 05:08 PDT, Angelos Oikonomopoulos
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Angelos Oikonomopoulos 2022-04-05 09:03:31 PDT
[JSC] Report test flakiness to resultsdb
Comment 1 Angelos Oikonomopoulos 2022-04-05 09:04:54 PDT
Created attachment 456704 [details]
Patch
Comment 2 Angelos Oikonomopoulos 2022-04-05 09:19:42 PDT
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.
Comment 3 Angelos Oikonomopoulos 2022-04-07 05:32:50 PDT
Created attachment 456912 [details]
Patch
Comment 4 Angelos Oikonomopoulos 2022-04-08 01:27:57 PDT
(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.
Comment 5 Jonathan Bedard 2022-04-08 16:15:01 PDT
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.
Comment 6 Angelos Oikonomopoulos 2022-04-11 02:22:41 PDT
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;
Comment 7 Angelos Oikonomopoulos 2022-04-11 02:32:08 PDT
(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.
Comment 8 Angelos Oikonomopoulos 2022-04-11 05:08:07 PDT
Created attachment 457248 [details]
Full report with some artificially introduced flakiness
Comment 9 Angelos Oikonomopoulos 2022-04-11 05:10:51 PDT
(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.
Comment 10 Jonathan Bedard 2022-04-11 10:20:55 PDT
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.
Comment 11 Angelos Oikonomopoulos 2022-04-12 06:59:10 PDT
(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.
Comment 12 Radar WebKit Bug Importer 2022-04-12 09:04:17 PDT
<rdar://problem/91629460>
Comment 13 Jonathan Bedard 2022-04-12 09:23:29 PDT
(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.
Comment 14 Angelos Oikonomopoulos 2022-04-12 09:28:30 PDT
Glad we found a plausible explanation for this!
Comment 15 EWS 2022-04-12 10:06:03 PDT
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].