Bug 238807

Summary: [resultsdb] Change termination condition to allow hashes in test results
Product: WebKit Reporter: Angelos Oikonomopoulos <angelos>
Component: New BugsAssignee: Angelos Oikonomopoulos <angelos>
Status: RESOLVED WONTFIX    
Severity: Normal CC: jbedard
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch jbedard: review-

Description Angelos Oikonomopoulos 2022-04-05 09:06:50 PDT 
[resultsdb] Change termination condition to allow hashes in test results
Comment 1 Angelos Oikonomopoulos 2022-04-05 09:10:09 PDT 
Created attachment 456706 [details]
Patch
Comment 2 Jonathan Bedard 2022-04-05 12:32:10 PDT 
Comment on attachment 456706 [details]
Patch

I want to deploy this to our staging instance before landing.
Comment 3 Jonathan Bedard 2022-04-05 14:32:17 PDT 
(In reply to Jonathan Bedard from comment #2)
> Comment on attachment 456706 [details]
> Patch
> 
> I want to deploy this to our staging instance before landing.

This would be why we deploy things to the staging instance:

Looking at https://results.webkit.org/api/upload?suite=layout-tests&limit=1, looks like we don't always specify the "actual" result if a test passes. So our check needs to be a bit more sophisticated, I think. Probably the old check, plus an extra check that recognizes that a set of keys that can be nested?
Comment 4 Angelos Oikonomopoulos 2022-04-08 01:12:13 PDT 
(In reply to Jonathan Bedard from comment #3)
> (In reply to Jonathan Bedard from comment #2)
> > Comment on attachment 456706 [details]
> > Patch
> > 
> > I want to deploy this to our staging instance before landing.
> 
> This would be why we deploy things to the staging instance:
> 
> Looking at https://results.webkit.org/api/upload?suite=layout-tests&limit=1,
> looks like we don't always specify the "actual" result if a test passes. So
> our check needs to be a bit more sophisticated, I think. Probably the old
> check, plus an extra check that recognizes that a set of keys that can be
> nested?

Thanks for the extra testing! I've looked at the layout tests code and I think it's probably safer in the long run to keep the current termination check. I've modified https://bugs.webkit.org/show_bug.cgi?id=238806 to use two new fields in the existing test result instead of a nested dict.