Bug 195404 – Run unit tests on remote host

• Component: New Bugs
• Status: NEW

[Dominik Inführ]

Run unit tests on remote machine

[Dominik Inführ]

Created attachment 363860 [details]
Patch

[Caio Lima]

Comment on attachment 363860 [details]
Patch

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

LGTM. I think it is a good idea to put on ChangeLog one example of how we can run these tests on command line. It will help a lot for future reference.

> Tools/ChangeLog:9
> +        with cross-compiled builds. Make the unit tests and JS files

I think we can change "Make the unit tests and JS files..." => "To enable remote run, we are making unit tests and JS files...".

> Tools/ChangeLog:14
> +        Avoid using `ldd` for retrieving JSC's dependencies, this does

I would use "..retrieving JSC's dependencies, since this does not work..."

> Tools/ChangeLog:16
> +        dependencies since changes should be quite rare anyway.

I would say: "As an alternative, we hardcode dependencies, since changes there are quite rare."

[Dominik Inführ]

Created attachment 363873 [details]
Patch

[Alexey Proskuryakov]

This patch implements running JSC tests on a separate device, which we already do for iOS. Is there duplication of functionality here?

[Dominik Inführ]

Yes, run-javascriptcore-tests (or actually run-jsc-stress-tests) runs the JS test files already on the remote machine. However the unit tests (testmasm, testb3, testair, testapi) are always executed locally though. This doesn't work in the case of cross compilation (we build on x64 and then run tests on one of our ARM boards).

[Dominik Inführ]

Created attachment 364361 [details]
Patch

[Dominik Inführ]

Created attachment 365005 [details]
Patch

[Dominik Inführ]

Created attachment 366169 [details]
Patch

[Dominik Inführ]

Updated to also execute testdfg.

[Yusuke Suzuki]

Comment on attachment 366169 [details]
Patch

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

> Tools/Scripts/run-jsc-stress-tests:126
> +$runTestMasm = false
> +$runTestAir = false
> +$runTestB3 = false
> +$runTestDFG = false
> +$runTestAPI = false

Hmm, I don't think this is the correct way. "run-jsc-stress-tests" are scripts for stress tests in JSC, and the stress tests do not include "testmasm" etc. Is it right?

[Dominik Inführ]

I think it makes a lot of sense to implement this in run-jsc-stress-tests since we can reuse all the code that handles remotes, bundles/deploys files and runs binaries on the remote machine.

I agree, run-jsc-stress-tests didn't run testmasm, etc. yet. However one could argue that it already runs more than just stress tests in JSTests/stress. Personally I see run-javascriptcore-tests as a wrapper around run-jsc-stress-tests.

[Guillaume Emont]

@Yusuke: in light of Dominik's last comment, do you still think this should be implemented differently? If yes, do you have a suggestion of what you think the right approach would be?

Also, I think a big argument for doing this in run-jsc-stress-tests is that it's where the whole remote logic is implemented. When parameters to run things remotely are passed to run-javascriptcore-tests, all it does is forward that to run-jsc-stress-tests.

[Guillaume Emont]

Created attachment 379655 [details]
Patch

Updated version of the patch that works with latest upstream and testdfg. Also fixes issues when running unit tests but no js tests on a remote host (run-jsc-stress-tests wouldn't be called in that case).

[Guillaume Emont]

(In reply to Guillaume Emont from comment #13)
> Created attachment 379655 [details]
> Patch
> 
> Updated version of the patch that works with latest upstream and testdfg.
> Also fixes issues when running unit tests but no js tests on a remote host
> (run-jsc-stress-tests wouldn't be called in that case).

...Also, I kept the changes where they are, using run-jsc-stress-tests to run the unit tests. While this is a little unexpected, the alternative seems to be to duplicate the remote testing functionality, by adding a second implementation of it in perl in run-javascriptcore-tests over the existing one in ruby in run-jsc-stress-tests, so I would think that the current solution is preferable.

[Caio Lima]

(In reply to Guillaume Emont from comment #14)
> (In reply to Guillaume Emont from comment #13)
> > Created attachment 379655 [details]
> > Patch
> > 
> > Updated version of the patch that works with latest upstream and testdfg.
> > Also fixes issues when running unit tests but no js tests on a remote host
> > (run-jsc-stress-tests wouldn't be called in that case).
> 
> ...Also, I kept the changes where they are, using run-jsc-stress-tests to
> run the unit tests. While this is a little unexpected, the alternative seems
> to be to duplicate the remote testing functionality, by adding a second
> implementation of it in perl in run-javascriptcore-tests over the existing
> one in ruby in run-jsc-stress-tests, so I would think that the current
> solution is preferable.

How about refactoring the remote execution out of `run-jsc-stress-tests` and share it with a new script to run unit tests? I think this would be a better approach than implementing it into `run-jsc-stress-tests` or duplicating the code into `run-javascriptcore-tests`.

[Guillaume Emont]

(In reply to Caio Lima from comment #15)
> (In reply to Guillaume Emont from comment #14)
> > (In reply to Guillaume Emont from comment #13)
> > > Created attachment 379655 [details]
> > > Patch
> > > 
> > > Updated version of the patch that works with latest upstream and testdfg.
> > > Also fixes issues when running unit tests but no js tests on a remote host
> > > (run-jsc-stress-tests wouldn't be called in that case).
> > 
> > ...Also, I kept the changes where they are, using run-jsc-stress-tests to
> > run the unit tests. While this is a little unexpected, the alternative seems
> > to be to duplicate the remote testing functionality, by adding a second
> > implementation of it in perl in run-javascriptcore-tests over the existing
> > one in ruby in run-jsc-stress-tests, so I would think that the current
> > solution is preferable.
> 
> How about refactoring the remote execution out of `run-jsc-stress-tests` and
> share it with a new script to run unit tests? I think this would be a better
> approach than implementing it into `run-jsc-stress-tests` or duplicating the
> code into `run-javascriptcore-tests`.

I'm giving it a go, though there's a bit of work, since the whole script relies heavily on global variables (and I'm learning ruby as I go).

[Guillaume Emont]

Created attachment 380214 [details]
Patch

I've moved the remote stuff to a new file, but I'm hitting a point where we might need a small discussion: I can either put the code to run remote unit tests in its own ruby script (say, run-jsc-remote-unit-tests), and leave the (perl) code to run unit tests locally as is in run-javascriptcore-tests; or alternatively, I can create a new run-jsc-unit-tests ruby script that would run unit tests both locally and remotely, thus avoiding code duplication. My preference goes towards the second solution that seems cleaner, but I'd like to make sure reviewers agree with me, as this will involve a fair amount of work, as I would port the unit test running from perl (in run-javascriptcore-tests) to ruby (in that new script), and it will be some work to test that things work properly on all platforms after the change.