Make run-javascript-core-test and run-jsc-stress-tests support a customized identity file

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Zhifei Fang]

Make run-javascript-core-test and run-jsc-stress-tests support a customized identity file

[Zhifei Fang]

Created attachment 442758 [details]
Patch

[Zhifei Fang]

Created attachment 442772 [details]
Patch

[Zhifei Fang]

Created attachment 442999 [details]
Patch

[Zhifei Fang]

Hi, Angelos,

Are those known failure? 


wasm.yaml/wasm/wast-tests/harness.js.(br-if-at-end-of-block.wasm)-wasm-no-tls-context: Exception: ReferenceError: Can't find variable: WebAssembly
wasm.yaml/wasm/wast-tests/harness.js.(br-if-at-end-of-block.wasm)-wasm-no-tls-context: global [e-mail]:6:45
wasm.yaml/wasm/wast-tests/harness.js.(br-if-at-end-of-block.wasm)-wasm-no-tls-context: ERROR: Unexpected exit code: 3
FAIL: wasm.yaml/wasm/wast-tests/harness.js.(br-if-at-end-of-block.wasm)-wasm-no-tls-context
Running wasm.yaml/wasm/wast-tests/harness.js.(osr-entry-inner-loop-branch-above-no-consts.wasm)-wasm-no-tls-context

[Angelos Oikonomopoulos]

(In reply to Zhifei Fang from comment #5)
> Hi, Angelos,
> 
> Are those known failure? 
> 
> 
> wasm.yaml/wasm/wast-tests/harness.js.(br-if-at-end-of-block.wasm)-wasm-no-
> tls-context: Exception: ReferenceError: Can't find variable: WebAssembly
> wasm.yaml/wasm/wast-tests/harness.js.(br-if-at-end-of-block.wasm)-wasm-no-
> tls-context: global [e-mail]:6:45
> wasm.yaml/wasm/wast-tests/harness.js.(br-if-at-end-of-block.wasm)-wasm-no-
> tls-context: ERROR: Unexpected exit code: 3
> FAIL:
> wasm.yaml/wasm/wast-tests/harness.js.(br-if-at-end-of-block.wasm)-wasm-no-
> tls-context
> Running
> wasm.yaml/wasm/wast-tests/harness.js.(osr-entry-inner-loop-branch-above-no-
> consts.wasm)-wasm-no-tls-context

No. This has exactly the same symptoms as the bug fixed in https://bugs.webkit.org/show_bug.cgi?id=226009. And indeed, if you check the logs you can see that run-jsc-stress-tests is getting run with --arch 0 which is obviously an invalid architecture name. Though I have no idea where that zero is coming from. AFAICT this is not happening to concurrent EWS runs for mips, so perhaps it's a side effect of the patch?

[Angelos Oikonomopoulos]

Oh now that I skimmed the patch, I'd take a close look at the change in webkitdirs.pm.

[Angelos Oikonomopoulos]

(In reply to Angelos Oikonomopoulos from comment #7)
> Oh now that I skimmed the patch, I'd take a close look at the change in
> webkitdirs.pm.

Hi, in case it helps, the issue with the change in webkitdirs.pm

-            $output = `ssh -o NoHostAuthenticationForLocalhost=yes -p $port $target 'uname  -m'`;
+            my $cmd = 'ssh -o NoHostAuthenticationForLocalhost=yes '. (exists $remote->{'idFilePath'} ? ('-i '.$remote->{'idFilePath'}) : '') ." -p $port $target 'uname  -m'";
+            $output = system($cmd);

seems to be that the new code calls system(), so it doesn't actually capture the command output (it just runs it) and it appears reading from the uninitialized `my $output` variable gives back a zero.

[Angelos Oikonomopoulos]

@@ -276,6 +276,7 @@ GetoptLong.new(['--help', '-h', GetoptLong::NO_ARGUMENT],
         $remote = true
         uri = URI("ssh://" + arg)
         $remoteHosts << RemoteHost.new("default-#{$remoteHosts.length}", uri.user, uri.host, uri.port)
+        $remoteHosts[0].identity_file_path = '~/.ssh/id_rsa'
     when '--remote-config-file'
         $remoteConfigFile = arg

Apropos, is it a good idea to set a file path here? Isn't it simpler to not pass in `-i` if identity_file_path is nil. This way, we don't hardcode ssh's current default.

[Radar WebKit Bug Importer]

<rdar://problem/85031252>

[Zhifei Fang]

Created attachment 443575 [details]
Patch

[Zhifei Fang]

Created attachment 443643 [details]
Patch

[Ryan Haddad]

Comment on attachment 443643 [details]
Patch

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

> Tools/Scripts/run-jsc-stress-tests:352
> +    print $remoteHosts[0].identity_file_path

Is this print intentional?

[Zhifei Fang]

Created attachment 444118 [details]
Patch

[Zhifei Fang]

(In reply to Ryan Haddad from comment #13)
> Comment on attachment 443643 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=443643&action=review
> 
> > Tools/Scripts/run-jsc-stress-tests:352
> > +    print $remoteHosts[0].identity_file_path
> 
> Is this print intentional?

Yes, I have made it print only there is an identity file path.

[Zhifei Fang]

Created attachment 444325 [details]
Patch

[Zhifei Fang]

Created attachment 444412 [details]
Patch for landing

[EWS]

Committed r285875 (244301@main): <https://commits.webkit.org/244301@main>

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