Bug 233926 – [JSC] Unify default/noisy tests definition

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

[Angelos Oikonomopoulos]

[JSC] Unify default/noisy tests definition

[Angelos Oikonomopoulos]

Created attachment 446166 [details]
Patch

[Angelos Oikonomopoulos]

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).

[Angelos Oikonomopoulos]

Created attachment 446167 [details]
Patch

[Yusuke Suzuki]

Comment on attachment 446167 [details]
Patch

r=me

[EWS]

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].

[Radar WebKit Bug Importer]

<rdar://problem/86425398>

[Michael Catanzaro]

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

[Angelos Oikonomopoulos]

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

[Angelos Oikonomopoulos]

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

Michael, could you give this a try?

[Michael Catanzaro]

(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.

[Angelos Oikonomopoulos]

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

Fixed patch.

[Angelos Oikonomopoulos]

(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.

[Michael Catanzaro]

Thanks!

[Angelos Oikonomopoulos]

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

[Saam Barati]

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?

[Saam Barati]

(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.