Bug 207106 - Update style checker with new locations OS version checks are allowed
Summary: Update style checker with new locations OS version checks are allowed
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: Sam Weinig
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-02-02 18:31 PST by Sam Weinig
Modified: 2020-02-04 09:52 PST (History)
7 users (show)

See Also:


Attachments
Patch (2.31 KB, patch)
2020-02-02 18:32 PST, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (7.57 KB, patch)
2020-02-02 18:42 PST, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (7.93 KB, patch)
2020-02-02 18:59 PST, Sam Weinig
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 Weinig 2020-02-02 18:31:27 PST
Update style checker with new locations OS version checks are allowed
Comment 1 Sam Weinig 2020-02-02 18:32:37 PST Comment hidden (obsolete)
Comment 2 Sam Weinig 2020-02-02 18:34:37 PST
I'd like to update the unit tests as well but I am not clear on how to run them. 

Anyone know how to run the unit tests in Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py?
Comment 3 Sam Weinig 2020-02-02 18:42:06 PST Comment hidden (obsolete)
Comment 4 Sam Weinig 2020-02-02 18:42:53 PST
For the time being, I will use my friendly EWS bot.
Comment 5 Sam Weinig 2020-02-02 18:59:19 PST
Created attachment 389489 [details]
Patch
Comment 6 Alexey Proskuryakov 2020-02-02 19:33:21 PST
Locally, test-webkitpy will run this test among others.
Comment 7 Sam Weinig 2020-02-02 19:43:59 PST
(In reply to Alexey Proskuryakov from comment #6)
> Locally, test-webkitpy will run this test among others.

Cool. Is there a way to get it to just run the one I am interested in to iterate?
Comment 8 WebKit Commit Bot 2020-02-02 20:26:59 PST
Comment on attachment 389489 [details]
Patch

Clearing flags on attachment: 389489

Committed r255553: <https://trac.webkit.org/changeset/255553>
Comment 9 WebKit Commit Bot 2020-02-02 20:27:01 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2020-02-02 20:27:14 PST
<rdar://problem/59100260>
Comment 11 Alexey Proskuryakov 2020-02-02 23:13:25 PST
> Cool. Is there a way to get it to just run the one I am interested in to
> iterate?

test-webkitpy webkitpy.style.checkers.cpp_unittest.WebKitStyleTest.test_os_version_checks
Comment 12 Sam Weinig 2020-02-03 07:12:28 PST
(In reply to Alexey Proskuryakov from comment #11)
> > Cool. Is there a way to get it to just run the one I am interested in to
> > iterate?
> 
> test-webkitpy
> webkitpy.style.checkers.cpp_unittest.WebKitStyleTest.test_os_version_checks

Heh. Thanks. So many letters!
Comment 13 Jonathan Bedard 2020-02-03 07:38:14 PST
(In reply to Sam Weinig from comment #7)
> (In reply to Alexey Proskuryakov from comment #6)
> > Locally, test-webkitpy will run this test among others.
> 
> Cool. Is there a way to get it to just run the one I am interested in to
> iterate?

Alexey's response here is correct, but it's worth noting that the tests are pretty quick (10-15 seconds) and we work pretty hard to keep them green. There are also integration tests, so it's not immediately obvious which tests will exercise new code.
Comment 14 Alexey Proskuryakov 2020-02-03 09:25:14 PST
> Heh. Thanks. So many letters!

It's kind of like a test path, just dot separated :). Once it fails for the first time, you can copy/paste it from the result.

CC Aakash Jain to consider whether we should preserve the leading "webkitpy.' everywhere in EWS - currently it is omitted, making it not straightforward to figure out how to run the subtest individually.
Comment 15 Jonathan Bedard 2020-02-03 11:24:00 PST
(In reply to Alexey Proskuryakov from comment #14)
> > Heh. Thanks. So many letters!
> 
> It's kind of like a test path, just dot separated :). Once it fails for the
> first time, you can copy/paste it from the result.
> 
> CC Aakash Jain to consider whether we should preserve the leading
> "webkitpy.' everywhere in EWS - currently it is omitted, making it not
> straightforward to figure out how to run the subtest individually.

We definitely need to preserve webkitpy in EWS, there are a small subset of tests run by test-webkitpy that are NOT prefixed by webkitpy.
Comment 16 Aakash Jain 2020-02-03 12:21:02 PST
> CC Aakash Jain to consider whether we should preserve the leading
> "webkitpy.' everywhere in EWS - currently it is omitted.
I removed it for readability. I will add it back.
Comment 17 Aakash Jain 2020-02-04 09:52:12 PST
(In reply to Aakash Jain from comment #16)
> I removed it for readability. I will add it back.
Fixing in https://bugs.webkit.org/show_bug.cgi?id=207206