Bug 221186 - Detect unrecognized options in run-javascriptcore-tests
Summary: Detect unrecognized options in run-javascriptcore-tests
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: Nobody
URL:
Keywords: InRadar
Depends on: 222320 222321 222325
Blocks:
  Show dependency treegraph
 
Reported: 2021-01-31 06:49 PST by Angelos Oikonomopoulos
Modified: 2021-03-04 05:01 PST (History)
7 users (show)

See Also:


Attachments
Patch (2.08 KB, patch)
2021-01-31 06:56 PST, Angelos Oikonomopoulos
no flags Details | Formatted Diff | Diff
Patch (2.09 KB, patch)
2021-02-02 01:53 PST, Angelos Oikonomopoulos
no flags Details | Formatted Diff | Diff
Patch (2.06 KB, patch)
2021-03-02 09:20 PST, Angelos Oikonomopoulos
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Angelos Oikonomopoulos 2021-01-31 06:49:46 PST
Detect unrecognized options in run-javascriptcore-tests
Comment 1 Angelos Oikonomopoulos 2021-01-31 06:56:30 PST
Created attachment 418825 [details]
Patch
Comment 2 Keith Miller 2021-02-01 15:47:04 PST
Comment on attachment 418825 [details]
Patch

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

> Tools/ChangeLog:12
> +        through to build-jsc even when --no-build is used. However, when
> +        we're not building nothing will ever use or look at the extra
> +        arguments, which means that those arguments are silently eaten
> +        up. This means that typos in option names can go undetected.

Seems like a run-on sentence. Maybe it would be clearer as: 

However, when we're not building, nothing will ever use or look at the extra arguments. This means that those arguments are silently eaten up.

> Tools/Scripts/run-javascriptcore-tests:490
> +    # Assume any arguments left over from GetOptions are assumed to be build arguments

Seems like there's an existing typo. Can you change this to:

Assume any arguments left over from GetOptions are to be passed as build arguments.
Comment 3 Angelos Oikonomopoulos 2021-02-02 01:53:36 PST
Created attachment 418973 [details]
Patch
Comment 4 Angelos Oikonomopoulos 2021-02-02 02:01:24 PST
Thanks -- the new patch includes the wording fixes.
Comment 5 Radar WebKit Bug Importer 2021-02-07 06:50:14 PST
<rdar://problem/74072601>
Comment 6 Angelos Oikonomopoulos 2021-02-23 02:58:36 PST
Ping.
Comment 7 Keith Miller 2021-02-23 08:14:54 PST
r=me
Comment 8 EWS 2021-02-23 08:18:14 PST
Committed r273307: <https://commits.webkit.org/r273307>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 418973 [details].
Comment 9 WebKit Commit Bot 2021-02-23 10:09:39 PST
Re-opened since this is blocked by bug 222320
Comment 10 Keith Miller 2021-02-23 11:04:01 PST
I think https://bugs.webkit.org/show_bug.cgi?id=222325 should fix the issue with the Cloop bot but I don't know how to test it locally.
Comment 11 Angelos Oikonomopoulos 2021-03-01 07:02:07 PST
(In reply to Keith Miller from comment #10)
> I think https://bugs.webkit.org/show_bug.cgi?id=222325 should fix the issue
> with the Cloop bot but I don't know how to test it locally.

Now that 222325 is in, perhaps we can try landing this patch again?
Comment 12 Keith Miller 2021-03-01 09:36:43 PST
(In reply to Angelos Oikonomopoulos from comment #11)
> (In reply to Keith Miller from comment #10)
> > I think https://bugs.webkit.org/show_bug.cgi?id=222325 should fix the issue
> > with the Cloop bot but I don't know how to test it locally.
> 
> Now that 222325 is in, perhaps we can try landing this patch again?

Not sure if cq+ing again works. You might need to upload a new copy of the patch. :/
Comment 13 EWS 2021-03-01 09:36:52 PST
ChangeLog entry in Tools/ChangeLog contains OOPS!.
Comment 14 EWS 2021-03-01 09:41:16 PST
commit-queue failed to commit attachment 418973 [details] to WebKit repository. To retry, please set cq+ flag again.
Comment 15 Angelos Oikonomopoulos 2021-03-02 09:20:20 PST
Created attachment 421948 [details]
Patch
Comment 16 Angelos Oikonomopoulos 2021-03-02 09:21:27 PST
(In reply to Keith Miller from comment #12)
[...]
> Not sure if cq+ing again works. You might need to upload a new copy of the
> patch. :/

Done!
Comment 17 EWS 2021-03-02 14:42:35 PST
commit-queue failed to commit attachment 421948 [details] to WebKit repository. To retry, please set cq+ flag again.
Comment 18 Angelos Oikonomopoulos 2021-03-04 02:30:20 PST
(In reply to EWS from comment #17)
> commit-queue failed to commit attachment 421948 [details] to WebKit
> repository. To retry, please set cq+ flag again.

I'm confused. push-commit-to-webkit-repo fails with

ERROR from SVN:
Item is out of date: File '/trunk/Source/WebKit/ChangeLog' is out of date
W: bd89e2521103c3f47dab717e6498ff947b9f9b29 and refs/remotes/origin/main differ, using rebase:
:040000 040000 ab97c4dac00c0354e4cd576430397ca9d44cc3ed 91dc1b3ad7a9b794b9f5036399ea0139abec72c1 M	Tools

But the patch doesn't touch WebKit/ChangeLog.
Comment 19 EWS 2021-03-04 05:01:05 PST
Committed r273883: <https://commits.webkit.org/r273883>

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