Bug 217390 – Refactor TestOptions code in WebKitTestRunner to make it easier to rationalize and extend

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Sam Weinig]

Refactor TestOptions code in WebKitTestRunner to make it easier to rationalize and extend

[Sam Weinig]

Created attachment 410649 [details]
Patch

[Darin Adler]

Comment on attachment 410649 [details]
Patch

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

I approve the idea of using the standard library in our test tools. However I don’t know what specifically motivates doing more of that here while still having so much code using WTF.

> Tools/WebKitTestRunner/Options.cpp:116
> +static bool parseFeature(String featureString, std::unordered_map<std::string, bool>& features)

Funny that this takes String, not StringView or const String&

> Tools/WebKitTestRunner/TestOptions.cpp:101
> +enum class KeyType {

Perhaps : uint8_t?

[Sam Weinig]

(In reply to Darin Adler from comment #2)
> Comment on attachment 410649 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=410649&action=review
> 
> I approve the idea of using the standard library in our test tools. However
> I don’t know what specifically motivates doing more of that here while still
> having so much code using WTF.

I'm mostly trying to ween WebKitTestRunner off of WTF where there are stdlib equivalents we can use, but I haven't been super consistent. Going to do another pass where I do just that.

> 
> > Tools/WebKitTestRunner/Options.cpp:116
> > +static bool parseFeature(String featureString, std::unordered_map<std::string, bool>& features)
> 
> Funny that this takes String, not StringView or const String&

I made it take std::string_view :).

> 
> > Tools/WebKitTestRunner/TestOptions.cpp:101
> > +enum class KeyType {
> 
> Perhaps : uint8_t?

Done.

[Sam Weinig]

Created attachment 410730 [details]
Patch

[EWS]

Committed r268115: <https://trac.webkit.org/changeset/268115>

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

[Radar WebKit Bug Importer]

<rdar://problem/70031230>

[Karl Rackler]

Reverted r268115 for reason:

This reverts commit r268115 while investigating ews test failure.

Committed r268143: <https://trac.webkit.org/changeset/268143>

[Sam Weinig]

Created attachment 410790 [details]
Path

[Sam Weinig]

Created attachment 410848 [details]
Patch

[Sam Weinig]

I do not understand this iOS-WebKit2 failure. What in my change is affecting anti-aliasing results?

[Sam Weinig]

Created attachment 410872 [details]
Patch

[EWS]

Tools/Scripts/svn-apply failed to apply attachment 410872 [details] to trunk.
Please resolve the conflicts and upload a new patch.

[Sam Weinig]

Created attachment 410896 [details]
Patch

[EWS]

Committed r268238: <https://trac.webkit.org/changeset/268238>

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