Refactor dev/tests/js/run_js_tests.php

[Vinai]

All global functions in the CLI script have been moved into a new class JsTestDriver located within the dev/tests/js/framework/JsTestDriver.php file.
The goal of the refactoring is to make the code more reusable and maintainable and make the script run on OS X without killing running PHPStorm instances.
The script itself is now just an entry point to call functionality within the new class.
The public interface of the JsTestDriver class is:

• JsTestRunner::fromConfigFile()- static factory method
• runTests() - functionality formerly within the run_js_tests.php script
• buildJsTestRunnerConfig() - rebuild the jsTestDriver.conf file

The functionality of the script has been decomposed into smaller functions.

A new entry point script has been added, build_js_test_conf.php, to just
rebuild the test configuration, without executing JsTestDriver.

[Vinai]

According to https://travis-ci.org/magento/magento2/builds the static_phpcs testsuite has been broken by the commit a1ab120

Running the tests locally doesn't list the JsTestDriver class or the files run_js_tests.php or build_js_test_conf.php in the report.

Rebasing the branch on the latest develop HEAD just in case.

[Vinai]

Close and reopen PR to restart travis build.

[antonkril]

How is this class more maintainable? I agree with some refactorings but what's the benefit of having functions namespaced in class?

[Vinai]

Of course it s not mandatory, using global scope and functions is great, but it just felt like a way to bind the methods to the config data set without using global variables or always passing the array as an argument.

I find smaller methods generally easier to understand and refactor.
To hide the larger number of methods, the class only exposes two public methods, which makes it easier to use, and also document the purpose.

Also, I find it easier to refactor code if I can see which methods are private, that is, implementation details, so they are safe to change.

I expect most of the command line scripts in Magento to be refactored sooner or later to use a common entry point (simply because that is what most PHP devs know from various frameworks).
Having the code in a class might make it easier to reuse some of it in that case.

I've also written some hacky tests (not commited) which compared the output of the class to the output of the originial script, and that also was easier to do with this version of the code in a class.

[antonkril]

Moving scripting behavior to class doesn't help. Also because all of the helper methods are private and on the same class, it's impossible to configure it differntly. Also different entry points should get different instances of different classes. writeJsTestRunnerConfig() has nothing to do with runTests()

[Vinai]

As stated in the commit message I didn't see this as the final result, it just was some refactoring along the way, to leave the code in a better state then it was. Decomposing the class would probably be one of the next steps.
For now it got me where I wanted, that is, understand what the code does and make future changes easier.

I actually don't expect this code to hang around for long. Based on @vkorotun's comments in #954 I expect the JS testing framework will change quite a lot, maybe completely removing JsTestDriver (I hope so).

[vkorotun]

Hi @Vinai,
We have started working on new JavaScript testing framework (based on Jasmine), and few days ago pushed first portion of the tests (documentation will follow up soon). New tests are placed under dev/tests/js/spec/unit and dev/tests/js/spec/integration paths for Unit and Integration tests correspondingly.

Short description of the workflow:

Preconditions:

• NodeJS is installed and runs
• NPM is installed (often installed by NodeJS itself)
• Grunt is installed

To install and configure environment, run these two commands in project root:
npm install -g grunt grunt-cli
npm install

To run unit tests:
grunt spec unit

To run integration tests:
grunt spec integration

[vpelipenko]

@Vinai, JavaScript testing framework changes mentioned by @vkorotun will be delivered with the next public GitHub update at the end of this week.

[vpelipenko]

@Vinai, the latest 0.42.0-beta7 version includes new JavaScript testing framework (based on Jasmine).

[Vinai]

Thank you so much for the updates. I've moved house and still don't have internet there (in hotel currently) so I was unable to participate any further. I'm looking forward to trying out the new JS testing framework 👍

[vpelipenko]

@Vinai, if you have any updates, please, share them with us.

[vpelipenko]

Closed due to publishing new JS testing framework and contributor's inactivity during 2 weeks. @Vinai, if you want to add something, please, feel free to reopen this PR or create new one.