Allow run-tests.sh to run just the javascript Functional tests

Do we need some kind of test here?

This test case looks fine to me.

Looks good just have to uncomment some code.

Thank you. This is ready now. Do we need a change notice for this?

Are we sure that this is the best way to go? Can't we use a special namespace like we're done with PHPUnit tests to get them to be run? And aren't we going to need to exclude these tests from a usual run?

I think we need to broaden this issue to do a bit more. Once we have javascript testing we have three test environments... simpletest, phpunit, and javascript (or maybe these are runners). Further more each environment can run different test types and these different types have different requriements:

• simpletest has both web tests and kernel tests
• phpunit has unit, kernel and functional
• javascript has ?

I think as a start we need to be able to define which test environments we're running so the following works as expected...

php run-tests.sh --testenv=simpletest,phpunit // runs simptest tests and phpunit tests
php run-tests.sh --testenv=phpunit // runs only phpunit tests
php run-tests.sh --testenv=javascript,phpunit // runs javascript and phpunit tests
php run-tests.sh --testenv=javascript // runs only javascript tests
php run-tests.sh --testenv=javascript,simpletest,phpunit // runs javascript, simpletest and phpunit tests
php run-tests.sh // runs all tests which atm means javascript, simpletest and phpunit tests

• simpletest has both web tests and kernel tests
• phpunit has unit, kernel and functional
• javascript has ?

We are not adding javascript tests here. We are adding javascript functional tests support afaik. Why would we use php to write JS test? We should use mocha, jUnit or etc. for that.
We'll run functional tests on Phantomjs which is merely another mink driver just like goutte or selenium. So to put it correctly

• simpletest has both web tests and kernel tests
• phpunit has unit, kernel and functional. Functional tests can be run on different mink drivers

Maybe IS doesn't explain completely what we are doing here. Unless I'm missing something.

I haven't been following the JS testing stuff, and I don't know much about JS testing, so I have a few naive questions:

Are we supporting core tests here, or adding options so that others can do stuff?

It looks like phantomjs has to be installed somewhere to run these, so how do we check for the dependency in the runner?

Do these tests fail or skip if the dependencies aren't met?

Do we have a reporting layer to talk back to the testbot and eventually d.o?

If there were a change notice or some other documentation to link to, that would help.

Maybe a crazy suggestion, what about reading on them first?

I did, which is why I asked those questions.

Last I looked the mink-based testing setup was basically experimental, which is why I'm wondering about whether this is for core or not. So I'm basically echoing @alexpott: "javascript has ?"

The answer seems to be that it's the same mink-flavored testing stuff, just with a JS head, so yay. Now: What happens if that infrastructure isn't present?

And here's a review:

1. +++ b/core/modules/simpletest/src/TestDiscovery.php
   @@ -345,6 +345,9 @@ public static function getTestInfo($classname, $doc_comment = NULL) {
   +    if (isset($annotations['javascript'])) {
   
   +++ b/core/modules/simpletest/tests/src/Unit/TestJavascriptTest.php
   @@ -0,0 +1,22 @@
   + * @javascript
   

   Call it @javascript-functional or something, since we'll eventually have JS unit tests.

2. +++ b/core/scripts/run-tests.sh
   @@ -208,6 +208,8 @@ function simpletest_script_help() {
   +  --javascript Runs the functional javascript tests.
   
   @@ -293,6 +295,7 @@ function simpletest_script_parse_args() {
   +    'javascript' => FALSE,
   
   @@ -902,6 +905,12 @@ function simpletest_script_get_test_list() {
   +      if ($args['javascript']) {
   

   Same deal. We'll have pure JS tests later and we might run them from run-tests.sh. Rename to --js-functional or something?

   We should also have a --functional flag. I bet there's an issue for it.

Just playing devil's advocate here...

Should we instead be looking at a real php-based build tool, like phing or robo.

Taking the phing approach (cause thats what I'm most familiar with, no other reason) - this would mean:
The bot would stick a build.properties in the checkout containing the environment variables such as db connection and build-artifacts directory as needed.
Then it would run one command - phing - which would spit out artifacts into the nominated directory, which jenkins would then pick up.
But this one command would be made up of a number of phing calls (e.g. it would run phpcs, unit tests, functional tests, javascript tests, integration tests).
If you're running locally and want to run javascript tests or unit tests, or code-coverage for example:
phing javascript
phing unit
phing phpcs
phing code-coverage

And so on.

That way we wouldn't rely on run-tests.sh to run phpunit tests - we would instead run them direct with the phpunit runner and the junit xml results printer (which jenkins understands).

In addition, we don't need to remember all the flags required - phing -l gives you your options - want to see what commands a build is running? phing unit -verbose

The exit code of the phpunit script would bubble to the phing/robo runner and determine the build status and we'd get away from all the code in simpletest module that wraps phpunit.

I think this is the ultimate end-point of the new CI, and in that ultimate end-game, run-tests.sh should only run legacy simpletest tests - at least in my opinion.

There was a session on this at Prague 2013 by @boztek - https://prague2013.drupal.org/session/have-build-and-automate-it-phing.html

1. +++ b/core/modules/simpletest/src/TestDiscovery.php
   @@ -329,8 +328,8 @@ public static function getTestInfo($classname, $doc_comment = NULL) {
   -      $info['group'] = 'PHPUnit';
   ...
   +      $info['group'] = 'PHPUnit-' . $testsuite;
   

   Yes please. :-)

2. +++ b/core/modules/simpletest/src/TestDiscovery.php
   @@ -414,26 +413,31 @@ public static function parseTestClassAnnotations(\ReflectionClass $class) {
   -  public static function isUnitTest($classname) {
   ...
   +  public static function getPhpunitTestSuite($classname) {
   

   Reminds me of this: #2605664: [Needs change record updates] Align TestDiscovery and bootstrap.php's non-permissive loading of PSR-4 namespaces for traits

I think this approach is okay. Whilst I'm not wild about changing groups of everything it is the best compromise for progress.

+++ b/core/modules/simpletest/src/TestDiscovery.php
@@ -329,8 +328,8 @@ public static function getTestInfo($classname, $doc_comment = NULL) {
     // Force all PHPUnit tests into the same group.

This comment needs updating

I think patch includes #2469713: Step 2: Create a JavaScriptTestBase using PhantomJs Driver/Binary now.

Sorry forgot to RTBC earlier.

So this patch makes the groups displayed in Simpletest UI less useful - we have the problem that we still only support one group per test. We should have addressed this when we added the PHPUnit group but we never did. I think we should address #2296615: Accurately support multiple @groups per test class first BUT it is super annoying to hold back javascript testing. None of the compromises here feel good :(

So thinking about this some more I think we shouldn't have overridden the group concept for phpunit. It was a mistake to override this so people could run all the PHPunit tests simply through the UI. If you want to run all the PHPunit tests just use the phpunit runner from CLI.

So therefore I think we should just add a type key to the test information. That way we can repurpose #2670978: Allow to run just specific types when running all tests to add the ability to specify test types to run when using run-tests.sh.

@dawehner I'm sorry for the change of direction again and I tried to reach out on IRC before posting this.

+++ b/core/modules/simpletest/tests/src/Unit/TestInfoParsingTest.php
@@ -56,11 +59,25 @@ public function infoParserProvider() {
+        'type' => 'PHPUnit-Functional',

Can we quickly add PHPUnit-FunctionalJavascript test as well?

+++ b/core/modules/simpletest/tests/src/Unit/TestInfoParsingTest.php
@@ -232,4 +256,29 @@ public function testTestInfoParserMissingSummary() {
+  public function testGetPhpunitTestSuite($classname, $expected) {
+    $this->assertEquals($expected, TestDiscovery::getPhpunitTestSuite($classname));
+  }
...
+    $data['module-functionaljavascripttest'] = ['\Drupal\Tests\toolbar\FunctionalJavascript\ToolbarIntegrationTest', 'FunctionalJavascript'];
...
+    $data['core-functionaljavascripttest'] = ['\Drupal\FunctionalJavascriptTests\ExampleTest', 'FunctionalJavascript'];

@jibran imo that is covered already.

Right but once testing suite and other is testing provided info. Anyway if you don't feel the need then it's fine by me.

+++ b/core/modules/simpletest/tests/src/Unit/TestInfoParsingTest.php
@@ -232,4 +256,29 @@ public function testTestInfoParserMissingSummary() {
+  public function testGetPhpunitTestSuite($classname, $expected) {
...
+  public function providerTestGetPhpunitTestSuite() {

This is getting test suite not phpunittest suite.

Committed/pushed to 8.1.x and cherry-picked to 8.0.x, thanks!

Some of the tests added here are broken: #2893371: Several methods theoretically added to TestInfoParsingTest were actually not