The files containing our automated tests were initially stored in /src/tests. But now that we have two test modules, in /tests/modules the actual test files should be moved into /tests/src. Probably they should be in /tests/src/functional as later we may introduce unit tests which would go into /tests/src/unit.

We also need to use convert the tests from WebTestBase to BrowserTestBase so that they can be run via PHPunit. See #2735005: Convert all Simpletest web tests to BrowserTestBase (or UnitTestBase/KernelTestBase)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jonathan1055 created an issue. See original summary.

jonathan1055’s picture

We can do the same thing in 7.x to keep all test-related files under one top-level directory.

Move scheduler.test into tests/scheduler.test and update the line in .info to say files[] = tests/scheduler.test. The three files .info, .install and .module for the scheduler_test module should be moved down to tests/modules

I have checked this and it works fine.


Edit: the 7.x changes are now on a separate issue #2837008: Move scheduler.test into /tests and move three test module into /tests/modules
jonathan1055’s picture

Title: Move test files from /src/tests to tests/src/functional » Move tests from /src/tests to tests/src/functional and extend BrowserTestBase instead of WebTestBase
Issue summary: View changes
Status: Active » Needs review
FileSize
8.43 KB

In moving to tests/src we also need to start using the BrowserTestBase class because WebTestBase will be discontinued as per #2735005: Convert all Simpletest web tests to BrowserTestBase (or UnitTestBase/KernelTestBase)

This patch creates two files: tests/src/Functional/SchedulerBrowserTestBase.php is just a copy of SchedulerTestBase.php with three changes:

  1. use Drupal\simpletest\WebTestBase<code> becomes <code>use Drupal\Tests\BrowserTestBase
  2. The namespace is changed from Drupal\scheduler\Tests to Drupal\Tests\scheduler\Functional
  3. The class SchedulerTestBase extends WebTestBase becomes SchedulerBrowserTestBase extends BrowserTestBase

The second file tests/src/Functional/SchedulerDefaultTimeTest.php is a trial conversion of the existing defaultTime test with changes similar to above:

  1. The namespace is changed from Drupal\scheduler\Tests to Drupal\Tests\scheduler\Functional
  2. The @group is now scheduler btb this is a temporary change to assist testing, ultimately it will remain as scheduler
  3. The lines extends SchedulerTestBase becomes extends SchedulerBrowserTestBase

There are improvements and changes we can do regarding assertion methods but currently all the existing assertions run the same as they do in SimpleTest. So the first exercise is just to move the files and use the new class.

It is not currently possible to execute cron runs in PHPunit tests due to #2795037: BTB: Add cronRun function so only a subset of our tests can be converted at this time.

  • jonathan1055 committed 3275832 on 8.x-1.x
    Issue #2824366 by jonathan1055: Move defaultTime test from /src/tests to...
jonathan1055’s picture

Four more test files moved.
APITest and DeleteNodeTest required no changes apart from the namespace and base class as above.

FieldsDisplayTest showed up a failing in WebTestBase where assertFieldByName() ignores the field value to check. BrowserTestBase does the check more thoroughly and hence showed up where we had previously put the wrong value in. I have also improved these checks by changing it from the 'weight' field to the 'type' field, as the weight can be shown or hidden depending on whether tabledrag is working. Using a field which is always displayed is clearer to understand and debug.

MetaInformation required a minor change from using assertHeader() to using assertSession()->responseHeaderEquals() instead. See #2795611: Provide a assertHeader legacy assertion

Status: Needs review » Needs work

The last submitted patch, 5: 2824366-5.move_tests_and_use_btb.patch, failed testing.

jonathan1055’s picture

The FieldsDisplay test passes OK locally using run-tests.sh and using PHPunit, and also via the web UI. However, all of this was at Drupal 8.2. However, in Drupal 8.3 the field edit-fields-scheduler-settings-type does not exist and is replaced by edit-fields-scheduler-settings-region hence the test failures.
The weight field is actually common to both 8.2 and 8.3 and is still used even if tabledrag is in operation. So it is safe to use that field and the tests should pass at both 8.2 and 8.3

  • jonathan1055 committed bc4ac87 on 8.x-1.x
    Issue #2824366 by jonathan1055: Convert API, DeleteNode, FieldsDisplay...
jonathan1055’s picture

This patch deals with another eight test files. The only code change required (apart from use and class name) is in the permissions test, where we had the same problem as before when using assertNoFieldByName() with a second parameter (field value). The WebTestBase version treats the empty string as "do not check the field value" but in BrowserTestBase an empty string is a valid field value, and for "do not test the value" the parameter needs to be NULL.

  • jonathan1055 committed 15e3689 on 8.x-1.x
    Issue #2824366 by jonathan1055: Convert PastDates, Permissions, Required...
jonathan1055’s picture

Here's a patch for the last two tests that can be converted for now. RulesActions did not need any changes, but RulesConditions required the caches to be cleared so that the status messages were shown when the different conditions were triggered.

The remaining five tests (FunctionalTest, LightweightCronTest, NodeAccessTest, NonEnabledTypeTest and RulesEventsTest) all use $this->cronRun() so will have to wait for #2795037: BTB: Add cronRun function

  • jonathan1055 committed a5d1b14 on 8.x-1.x
    Issue #2824366 by jonathan1055: Move RulesActions and RulesConditions...
jonathan1055’s picture

I decided that it was better to get the remaining five tests converted and complete this issue, so I have added function cronRun() to SchedulerBrowserTestBase, as a direct copy from the function in patch #31 in #2795037: BTB: Add cronRun function. When that issue lands we can remove our own version.

There were two ammendments to the test code which ran OK in WTB but failed in BTB:

  1. In LightweightCronTest the casting as string fails so (string)$key_xpath[0] has to be changed to $key_xpath[0]->getText()
  2. In NonEnabledTypeTest the second parameter of assertFieldByName() has to be changed from empty string '' to NULL

  • jonathan1055 committed 499d82a on 8.x-1.x
    Issue #2824366 by jonathan1055: Move Functional, Lightweight, NodeAccess...
jonathan1055’s picture

To avoid confusion now that we have functional tests in the /Functional folder, I have renamed the original test file called SchedulerFunctionalTest.php to be SchedulerBasicTest.php

  • jonathan1055 committed 93173a4 on 8.x-1.x
    Issue #2824366 by jonathan1055: Renamed SchedulerFunctionalTest.php to...
jonathan1055’s picture

Status: Needs review » Fixed

All 8.x test files have now been moved and all pass when using BrowserTestBase. Separate issues can be raised to convert the various assertions which are being removed but that will not be a problem until 9.x development starts.

The 7.x follow-up is on #2837008: Move scheduler.test into /tests and move three test module into /tests/modules

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.