Bug 224563 - Store whether a test is slow on TestInput
Summary: Store whether a test is slow on TestInput
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sam Sneddon [:gsnedders]
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-04-14 11:12 PDT by Sam Sneddon [:gsnedders]
Modified: 2021-05-20 06:48 PDT (History)
5 users (show)

See Also:


Attachments
Patch (17.52 KB, patch)
2021-04-14 11:32 PDT, Sam Sneddon [:gsnedders]
no flags Details | Formatted Diff | Diff
Patch (34.03 KB, patch)
2021-05-19 16:18 PDT, Sam Sneddon [:gsnedders]
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Sneddon [:gsnedders] 2021-04-14 11:12:45 PDT
Bug 161626 introduced test-objects.json, however no test-objects.json files were ever added and this has thus been permanently dead code. Given it's lack of usage, let's remove it.

Additionally, with it gone, the expectations is once against the canonical source of knowledge about whether a test is slow and we don't need to communicate this through all the various components.
Comment 1 Sam Sneddon [:gsnedders] 2021-04-14 11:32:10 PDT
Created attachment 426022 [details]
Patch
Comment 2 Sam Sneddon [:gsnedders] 2021-04-15 07:22:35 PDT
Somehow I totally missed the file (did I look in Tools or something equally silly?), and this is in fact used. Hmm. I wonder what to do here instead; having two different places where we store this metadata (especially within webkitpy's data model) is somewhat clumsy and annoying.
Comment 3 Jonathan Bedard 2021-04-15 08:52:15 PDT
(In reply to Sam Sneddon [:gsnedders] from comment #2)
> Somehow I totally missed the file (did I look in Tools or something equally
> silly?), and this is in fact used. Hmm. I wonder what to do here instead;
> having two different places where we store this metadata (especially within
> webkitpy's data model) is somewhat clumsy and annoying.

Seems like all the effected tests are http tests....is there any reason we can't convert them to normal test expectations?
Comment 4 Radar WebKit Bug Importer 2021-04-21 11:13:14 PDT
<rdar://problem/76969468>
Comment 5 Sam Sneddon [:gsnedders] 2021-05-19 16:18:08 PDT
Created attachment 429110 [details]
Patch
Comment 6 Sam Sneddon [:gsnedders] 2021-05-19 16:21:10 PDT
(In reply to Jonathan Bedard from comment #3)
> (In reply to Sam Sneddon [:gsnedders] from comment #2)
> > Somehow I totally missed the file (did I look in Tools or something equally
> > silly?), and this is in fact used. Hmm. I wonder what to do here instead;
> > having two different places where we store this metadata (especially within
> > webkitpy's data model) is somewhat clumsy and annoying.
> 
> Seems like all the effected tests are http tests....is there any reason we
> can't convert them to normal test expectations?

We could, though there was concern about bloating TestExpectations with WPT before.

I think some of this is also related to the fact they'd need to be marked as slow in _every_ TestExpectations file, unless I'm misremembering how platform-specific expectations are added.

That said, this new patch does a bunch of refactoring that we need to do anyway, and with that done it becomes pretty easy to support tests-options.json again.
Comment 7 EWS 2021-05-20 06:48:50 PDT
Committed r277781 (237942@main): <https://commits.webkit.org/237942@main>

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