Bug 233926

Summary:

[JSC] Unify default/noisy tests definition

Product:

WebKit

Reporter:

Angelos Oikonomopoulos <angelos>

Component:

New Bugs

Assignee:

Angelos Oikonomopoulos <angelos>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aperez, don.olmstead, jbedard, keith_miller, mcatanzaro, ryanhaddad, saam, tzagallo, webkit-bug-importer, ysuzuki

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Try to fix the reported LocalJumpError	none
Try to fix the reported LocalJumpError	none

Description Angelos Oikonomopoulos 2021-12-07 06:02:29 PST

[JSC] Unify default/noisy tests definition

Comment 1 Angelos Oikonomopoulos 2021-12-07 06:07:55 PST

Created attachment 446166 [details]
Patch

Comment 2 Angelos Oikonomopoulos 2021-12-07 06:10:10 PST

Note that this patch is motivated by https://github.com/WebKit/WebKit/commit/f2460842a8e18a1dd509504c34acae44fdc8499e. The test is skipped on ARM because it's flaky in bytecode-cache mode, but changing it to use defaultRunNoisyTest drops bytecode-cache entirely (and seemingly unintentionally).

Comment 3 Angelos Oikonomopoulos 2021-12-07 06:17:30 PST

Created attachment 446167 [details]
Patch

Comment 4 Yusuke Suzuki 2021-12-13 11:16:53 PST

Comment on attachment 446167 [details]
Patch

r=me

Comment 5 EWS 2021-12-13 11:57:37 PST

Committed r286963 (245186@main): <https://commits.webkit.org/245186@main>

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

Comment 6 Radar WebKit Bug Importer 2021-12-13 11:58:21 PST

<rdar://problem/86425398>

Comment 7 Michael Catanzaro 2021-12-14 06:23:49 PST

It seems this commit broke the run-jsc-stress-tests script:

+ ./Tools/Scripts/run-jsc-stress-tests -c 1 --no-copy --jsc <http://redacted/WebKitBuild/Release/bin/jsc> --no-jit --memory-limited -v JSTests/stress
Warning: cannot identify JSC framework, doing generic VM copy.
./Tools/Scripts/run-jsc-stress-tests:1376:in `block in <main>': unexpected return (LocalJumpError)
	from ./Tools/Scripts/run-jsc-stress-tests:1422:in `block (3 levels) in <main>'
	from (eval):1:in `block (3 levels) in parseRunCommands'
	from ./Tools/Scripts/run-jsc-stress-tests:845:in `eval'
	from ./Tools/Scripts/run-jsc-stress-tests:845:in `block (3 levels) in parseRunCommands'
	from ./Tools/Scripts/run-jsc-stress-tests:835:in `each_line'
	from ./Tools/Scripts/run-jsc-stress-tests:835:in `block (2 levels) in parseRunCommands'
	from ./Tools/Scripts/run-jsc-stress-tests:833:in `open'
	from ./Tools/Scripts/run-jsc-stress-tests:833:in `block in parseRunCommands'
	from ./Tools/Scripts/run-jsc-stress-tests:832:in `chdir'
	from ./Tools/Scripts/run-jsc-stress-tests:832:in `parseRunCommands'
	from ./Tools/Scripts/run-jsc-stress-tests:2385:in `block (2 levels) in handleCollectionDirectory'
	from ./Tools/Scripts/run-jsc-stress-tests:2378:in `each'
	from ./Tools/Scripts/run-jsc-stress-tests:2378:in `block in handleCollectionDirectory'
	from ./Tools/Scripts/run-jsc-stress-tests:2376:in `chdir'
	from ./Tools/Scripts/run-jsc-stress-tests:2376:in `handleCollectionDirectory'
	from ./Tools/Scripts/run-jsc-stress-tests:2396:in `handleCollection'
	from ./Tools/Scripts/run-jsc-stress-tests:2492:in `block in prepareBundle'
	from ./Tools/Scripts/run-jsc-stress-tests:2490:in `each'
	from ./Tools/Scripts/run-jsc-stress-tests:2490:in `prepareBundle'
	from ./Tools/Scripts/run-jsc-stress-tests:3186:in `prepareArtifacts'
	from <redacted/Tools/Scripts/webkitruby/jsc-stress-test/executor.rb>:115:in `loop'
	from ./Tools/Scripts/run-jsc-stress-tests:3281:in `runNormal'
	from ./Tools/Scripts/run-jsc-stress-tests:3323:in `<main>'

Any chance the problem is obvious, or do I need to learn Ruby? :P

Comment 8 Angelos Oikonomopoulos 2021-12-14 06:39:34 PST

Oops. Can't reproduce it here but I'll have a patch for you shortly.

Comment 9 Angelos Oikonomopoulos 2021-12-14 06:42:59 PST

Created attachment 447127 [details]
Try to fix the reported LocalJumpError

Michael, could you give this a try?

Comment 10 Michael Catanzaro 2021-12-14 07:01:10 PST

(In reply to Angelos Oikonomopoulos from comment #8)
> Oops. Can't reproduce it here but I'll have a patch for you shortly.

Thanks, will try that. I guess it must be caused by --no-jit.

Comment 11 Angelos Oikonomopoulos 2021-12-14 07:03:53 PST

Created attachment 447129 [details]
Try to fix the reported LocalJumpError

Fixed patch.

Comment 12 Angelos Oikonomopoulos 2021-12-14 07:05:45 PST

(In reply to Michael Catanzaro from comment #10)
> (In reply to Angelos Oikonomopoulos from comment #8)
> > Oops. Can't reproduce it here but I'll have a patch for you shortly.
> 
> Thanks, will try that. I guess it must be caused by --no-jit.

Was a copy-paste error on my part; reproduced and confirmed that the patch fixes it.

Comment 13 Michael Catanzaro 2021-12-14 07:09:14 PST

Thanks!

Comment 14 Angelos Oikonomopoulos 2021-12-14 07:10:50 PST

Submitted separately in https://bugs.webkit.org/show_bug.cgi?id=234295.

Comment 15 Saam Barati 2022-04-04 16:45:58 PDT

Our tests everywhere are annotated with things like "//@ runDefault", how does this change not break those tests to now run in a different mode since you renamed the function?

Comment 16 Saam Barati 2022-04-04 16:47:46 PDT

(In reply to Saam Barati from comment #15)
> Our tests everywhere are annotated with things like "//@ runDefault", how
> does this change not break those tests to now run in a different mode since
> you renamed the function?

Nvm, I see we're dynamically defining these functions.