Bug 171627 - messages_unittest.py should support a [-r|--reset-results] switch
Summary: messages_unittest.py should support a [-r|--reset-results] switch
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: David Kilzer (:ddkilzer)
URL:
Keywords:
Depends on: 171510
Blocks: 250398
  Show dependency treegraph
 
Reported: 2017-05-03 15:37 PDT by David Kilzer (:ddkilzer)
Modified: 2023-01-10 11:10 PST (History)
12 users (show)

See Also:


Attachments
Patch v1 (53.36 KB, patch)
2017-05-03 15:40 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff
Patch v2 (56.11 KB, patch)
2017-05-04 05:37 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff
Patch v3 (56.01 KB, patch)
2017-05-04 05:39 PDT, David Kilzer (:ddkilzer)
dbates: review+
dbates: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews115 for mac-elcapitan (1.76 MB, application/zip)
2017-05-04 07:01 PDT, Build Bot
no flags Details
Patch v4 (57.06 KB, patch)
2017-05-05 04:35 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff
Patch v5 (to make title be grammatically consistent) (54.50 KB, patch)
2017-05-05 04:37 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2017-05-03 15:37:32 PDT
messages_unittest.py should support an [-u|--update] switch.
Comment 1 David Kilzer (:ddkilzer) 2017-05-03 15:40:56 PDT
Created attachment 308976 [details]
Patch v1
Comment 2 David Kilzer (:ddkilzer) 2017-05-03 15:50:07 PDT
Ah, this patch is blocked on the fix for Bug 171510 landing.
Comment 3 Sam Weinig 2017-05-03 17:35:04 PDT
Don't we call this --reset-results elsewhere? Or is this different from that?
Comment 4 David Kilzer (:ddkilzer) 2017-05-03 20:47:21 PDT
(In reply to Sam Weinig from comment #3)
> Don't we call this --reset-results elsewhere? Or is this different from that?

Yes, with run-webkit-tests.  Easy enough to change if that's preferred.
Comment 5 Sam Weinig 2017-05-03 23:18:54 PDT
(In reply to David Kilzer (:ddkilzer) from comment #4)
> (In reply to Sam Weinig from comment #3)
> > Don't we call this --reset-results elsewhere? Or is this different from that?
> 
> Yes, with run-webkit-tests.  Easy enough to change if that's preferred.

It's also what we use for run-binding-tests. Yeah, I think it would be better to be consistent.
Comment 6 David Kilzer (:ddkilzer) 2017-05-04 05:37:55 PDT
Created attachment 309035 [details]
Patch v2
Comment 7 David Kilzer (:ddkilzer) 2017-05-04 05:39:09 PDT
Created attachment 309036 [details]
Patch v3

Same as "Patch v2" but used --patience to create the patch.
Comment 8 Build Bot 2017-05-04 05:40:55 PDT
Attachment 309036 [details] did not pass style-queue:


ERROR: Source/WebKit2/Scripts/webkit/LegacyMessages-expected.h:42:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebKit2/Scripts/webkit/Messages-expected.h:42:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 2 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 David Kilzer (:ddkilzer) 2017-05-04 06:05:40 PDT
(In reply to Build Bot from comment #8)
> Attachment 309036 [details] did not pass style-queue:
> 
> 
> ERROR: Source/WebKit2/Scripts/webkit/LegacyMessages-expected.h:42:  Code
> inside a namespace should not be indented.  [whitespace/indent] [4]
> ERROR: Source/WebKit2/Scripts/webkit/Messages-expected.h:42:  Code inside a
> namespace should not be indented.  [whitespace/indent] [4]
> Total errors found: 2 in 8 files
> 
> 
> If any of these errors are false positives, please file a bug against
> check-webkit-style.

This means that the generated code doesn't pass style guidelines.
Comment 10 Daniel Bates 2017-05-04 06:10:31 PDT
Comment on attachment 309036 [details]
Patch v3

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

r=me

> Source/WebKit2/ChangeLog:8
> +        This change adds support for an [-r|--reset-reesults] switch

an => a
--reset-reesults => --reset-results

> Source/WebKit2/ChangeLog:52
> +        - Parses [-r|--reset-results] switch, and then remove it from

remove => removes

> Source/WebKit2/Scripts/webkit/messages_unittest.py:357
> +        if arg in ('-r', '--r', '--reset', '--reset-results') or '--reset-results'.startswith(arg):

Wow, this is pretty generous parsing. I wish there was a way to make this option discoverable.
Comment 11 Build Bot 2017-05-04 07:01:14 PDT
Comment on attachment 309036 [details]
Patch v3

Attachment 309036 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3672235

New failing tests:
fast/dom/Window/setTimeout-no-arguments.html
Comment 12 Build Bot 2017-05-04 07:01:16 PDT
Created attachment 309042 [details]
Archive of layout-test-results from ews115 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 13 David Kilzer (:ddkilzer) 2017-05-05 04:28:25 PDT
(In reply to Daniel Bates from comment #10)
> Comment on attachment 309036 [details]
> Patch v3
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=309036&action=review
> 
> r=me
> 
> > Source/WebKit2/Scripts/webkit/messages_unittest.py:357
> > +        if arg in ('-r', '--r', '--reset', '--reset-results') or '--reset-results'.startswith(arg):
> 
> Wow, this is pretty generous parsing. I wish there was a way to make this
> option discoverable.

Done in Patch v4!

This is what the help output looks like after this change:

$ python Source/WebKit2/Scripts/webkit/messages_unittest.py -h
Usage: messages_unittest.py [options] [test] [...]

Custom Options:
  -r, --reset-results  Reset expected results for messages_unittest.py

Options:
  -h, --help       Show this message
  -v, --verbose    Verbose output
  -q, --quiet      Minimal output
  -f, --failfast   Stop on first failure
  -c, --catch      Catch control-C and display results
  -b, --buffer     Buffer stdout and stderr during test runs

Examples:
  messages_unittest.py                               - run default set of tests
  messages_unittest.py MyTestSuite                   - run suite 'MyTestSuite'
  messages_unittest.py MyTestCase.testSomething      - run MyTestCase.testSomething
  messages_unittest.py MyTestCase                    - run all 'test*' test methods
                                               in MyTestCase
Comment 14 David Kilzer (:ddkilzer) 2017-05-05 04:35:13 PDT
Created attachment 309159 [details]
Patch v4

Changes from Patch v3:
- Changed use of split('\n') to more canonical split lines(False).
- Added add_reset_results_to_unittest_help() to add [-r|--reset-results] switch to help/usage output.
Comment 15 Build Bot 2017-05-05 04:37:02 PDT
Attachment 309159 [details] did not pass style-queue:


ERROR: Source/WebKit2/Scripts/webkit/LegacyMessages-expected.h:42:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebKit2/Scripts/webkit/Messages-expected.h:42:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 2 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 16 David Kilzer (:ddkilzer) 2017-05-05 04:37:19 PDT
Created attachment 309161 [details]
Patch v5 (to make title be grammatically consistent)
Comment 17 Build Bot 2017-05-05 04:39:57 PDT
Attachment 309161 [details] did not pass style-queue:


ERROR: Source/WebKit2/Scripts/webkit/LegacyMessages-expected.h:42:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebKit2/Scripts/webkit/Messages-expected.h:42:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 2 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 18 WebKit Commit Bot 2017-05-06 08:05:51 PDT
Comment on attachment 309161 [details]
Patch v5 (to make title be grammatically consistent)

Clearing flags on attachment: 309161

Committed r216327: <http://trac.webkit.org/changeset/216327>
Comment 19 WebKit Commit Bot 2017-05-06 08:05:53 PDT
All reviewed patches have been landed.  Closing bug.