Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New `run-tests.ts`, making test runs more reliable #1401

Merged
merged 21 commits into from Mar 26, 2019

Conversation

Projects
2 participants
@borekb
Copy link
Member

commented Mar 13, 2019

Issue: #1389
Issue: #1400

The main goal of this PR is to improve reliability of our test runs – npm run tests:full should produce consistent results across operating systems and several runs on the same machine.

Highlights:

  • New scripts/run-tests.ts instead of direct invocation of docker-compose and PHPUnit. This allows better control over how the tests run, which containers are started and in which order, there's proper waiting between them (c1a1fbb), etc. This resolves many confusing behaviors when the computer was too slow to start or something similar.
  • The script is smart enough to start or not to start WordPress depending on the test suite being run.
  • The whole Docker stack is now cleared before tests start. Previous runs cannot influence the current one.

Full TODO list:

  • Add scripts/run-tests.ts to orchestrate test runs.
  • Clear whole Docker stack before starting tests.
  • Imply WP container startup from specific test suites. For example, --testsuite Workflow should start WP.
  • Remove debugging scripts and info, reintroduce it later (we have some todo items related to Xdebug in #1389).
  • Add an argument to provide custom phpunit.xml
  • Remove tests:custom scripts from package.json.
  • Update docs in testing.md.
  • Fail immediately by default, flag to let them run I've decided against it for now.
  • On Ctrl+C to the Node.js script, gracefully shut down containers. Postponed; the user has to clean up manually.
  • Properly colorize PHPUnit output.
  • Pass through standard PHPUnit arguments
  • Simplify calling node -r ts-node/register run-tests ... to npm run tests -- ....

Noted for followup PRs:

  • Inspecting site via docker-compose -f docker-compose-tests.yml up -d wordpress-for-tests mysql-for-tests is a bit weird, review it.
  • Should we still have tests:cleanup?
Run workflow tests before end2end tests
This fixes `npm run tests:all` for me. Not the right solution but at least something until we find the true cause.

@borekb borekb self-assigned this Mar 13, 2019

@borekb borekb added the scope: tests label Mar 13, 2019

@borekb borekb added this to the 4.0 milestone Mar 13, 2019

@borekb

This comment has been minimized.

Copy link
Member Author

commented Mar 13, 2019

1778c7f shows how moving Workflow tests above End2End tests in phpunit.xml fixes #1400 (comment) for me.

However, it also makes tests run considerably slower – 30 minutes instead of 10 for npm run tests:full.

I think we should clean up the testing environment before each "big" test suite.

Add waiting container and a run-tests script (_draft_)
It's demonstrated on TemporaryDbTest that should fail if waiting is commented out from `run-tests.ts`.

@borekb borekb force-pushed the 1400-more-reliable-test-runs branch from 7e3f172 to c1a1fbb Mar 13, 2019

@borekb

This comment has been minimized.

Copy link
Member Author

commented Mar 13, 2019

c1a1fbb deals with another problem that @pavelevap frequently encounters – tests run before the Docker containers are fully started (WP + MySQL take around 20 seconds to start on my machine, can be longer depending on a specific device).

The commit shows an approach where a waiting container is added to docker-compose and a test running script uses it to delay the test execution. Key parts:

wait:
image: janvoracek/docker-wait@sha256:2d91ec68cf3e0dbcd03addaded0af238944f75378f5062358bd3c5fb839b60d2
environment:
- TARGETS=wordpress-for-tests:80,mysql-for-tests:3306

shell.exec(`${dc} run --rm wait`);

Running the commit as it is with node_modules/.bin/ts-node scripts/run-tests.ts works fine (notice the "Waiting" lines):

Screenshot 2019-03-14 at 00 20 53

But when I comment out the waiting line in run-tests.ts, I get this error:

Exception: Error executing cmd 'php '/tmp/wp-cli-latest-stable.phar' db query --dbuser='root' --dbpass='r00tpwd' 'SELECT User FROM mysql.user'' from working directory '/var/www/html/wptest':
ERROR 2002 (HY000): Can't connect to MySQL server on 'mysql' (115)

So that proves that the waiting container works.


Note that on my machine, to make the test fail, it was necessary to start the MySQL from an entirely clean state – with the volume deleted, otherwise, it was too quick to start to demonstrate the issue. That's why the run-tests.ts script first calls docker-compose down -v.

borekb added some commits Mar 16, 2019

Revert "Run workflow tests before end2end tests"
This will be resolved differently. Also, changing the order made tests run much slower (30 minutes instead of 10 on my machine).

This reverts commit 1778c7f.

borekb added some commits Mar 17, 2019

run-tests.ts can now run Unit tests without starting WP
zeit/arg is used to parse arguments, which currently are:

- `--help`
- `--with-wordpress` (false by default which is why unit tests don't start anything)
- `--testsuite` – an array of testsuites from phpunit.xml to run.
Run tests:full via run-tests.ts
The script added a detection of whether all tests will be run and sets `--with-wordpress` accordingly.
@borekb

This comment has been minimized.

Copy link
Member Author

commented Mar 17, 2019

The recent commits implemented run-tests.ts to a reasonable state where it can run both unit and all tests better than the raw npm scripts before:

Screenshot 2019-03-17 at 22 38 06

Screenshot 2019-03-17 at 22 37 00

The script has this "API" for now:

  Usage
    $ run-tests ...

  Options
    --testsuite         Testsuite from phpunit.xml to run. Can be repeated
                        multiple times.
    --with-wordpress    Start WordPress & MySQL containers. WIP: is implied
                        from --testsuite in some cases.
    -h, --help          Show help.

  Examples
    $ run-tests --testsuite Unit

TODO:

  • Imply --with-wordpress from specific test suites. For example, --testsuite Workflow should start WP.
  • Document how run-tests.ts relates to phpunit.xml.
  • Add an argument to provide custom phpunit.xml – this will allow us to remove tests:custom scripts from package.json.
  • Update public docs in testing.md.
@borekb

This comment has been minimized.

Copy link
Member Author

commented Mar 17, 2019

BTW I'm pretty happy with the test reliability now:

  • The tests wait for containers to start.
  • I get consistent results on both Mac and Docker Toolbox on Windows (documented in #1403).
  • The one failing test – #1400 (comment) – is probably a real bug, @pavelevap will be creating an issue for it.

The run-tests.ts scripts also provides foundation for future orchestration of tests outside of PHPUnit. For example, we'll be able to run full tests with WP-CLI worker and then with Selenium worker quite easily – this TypeScript script will simply run PHPUnit twice. Also, something like --clean-wp-before-each-testsuite can be added quite easily, I like how the test runs will be more customizable.

borekb added some commits Mar 19, 2019

Start MySQL before WordPress via full run-tests.ts orchestration
In a typical docker-compose setup, WordPress and MySQL start at the same time and the WordPress container then implements a logic to wait for MySQL to start, see [here](https://github.com/docker-library/wordpress/blob/acd229d6d638566a3ccd921d20075182db9e1185/php7.2/apache/docker-entrypoint.sh#L255-L266).

It "only" tries for 30 seconds, though, which is not enough on less powerful computers which leads to a confusing experience. See also docker-library/wordpress#392.

This commit implements a solution where MySQL starts first, we await its availability and only then start the WordPress container.

With this custmization, and with run-tests.ts in general, we don't really want services from `docker-compose-tests.yml` to be started manually so I added a big warning at the top of that file and removed all "links:" that would indicate that containers can start on their own – that's no longer true.

Also, the waiting container is created by plain `docker run` inside run-tests.ts instead of being an empty "service" inside the YAML. I'm a bit torn on this so let's see...
Infer whether WordPress is needed from testsuite
For example, End2End or Workflow tests start WordPress, unit tests don't.

As part of this, I simplified the run-tests script:

- `--with-wordpress` cannot be specified manually, it is always inferred.
- Only one `--testsuite` can be passed, not an array of them.

KISS.
Update testing.md – first pass after run-tests.ts
Quite a few things have been deleted as they no longer apply or are likely outdated.
Remove obsolete debugging scripts
`testing.md` now contains several smaller TODOs but the debugging information that I still think is valid is there.
Show resolved Hide resolved docker-compose-tests.yml Outdated

borekb added some commits Mar 22, 2019

`tests:full` → just `tests`; recommend using this to run custom tests
Intead of a complicated manual invocation via `node -r node_modules/ts-node/register`, it is now recommended to run custom scripts via `npm run tests -- <parameters>`.

@borekb borekb marked this pull request as ready for review Mar 23, 2019

@borekb

This comment has been minimized.

Copy link
Member Author

commented Mar 23, 2019

@pavelevap The PR is now ready for a review. Docs preview here.

@borekb

This comment has been minimized.

Copy link
Member Author

commented Mar 24, 2019

I'm getting consistent results between several machines now. 🎉

MacBook Pro with Docker Desktop

MacBook Pro (2015)
macOS Mochave 10.14.3
Intel Core i7 @ 2.5 GHz, 4 Cores, 8 Logical Processors
16 GB RAM
Docker Desktop for Mac 2.0.0.3 (build 31259, engine 18.09.2)

Tests took 11 minutes.

Screenshot 2019-03-24 at 20 49 51

Windows laptop with Docker Desktop

Dell XPS 13 9350 (2015)
Windows 10 v1809 ("October 2018 Update")
Core i5-6200U CPU @ 2.30GHz, 2 Cores, 4 Logical Processors
8 GB RAM
Docker Desktop for Windows 2.0.0.3 (build 8858db3, engine 18.09.2)

Tests took 18 minutes. (The CLI environment is using this MSYS2 setup.)

image

Windows laptop with Docker Toolbox

HP ProBook 650 G1 (2014)
Windows 10 v1809 ("October 2018 Update")
Core i7-4702MQ CPU @ 2.20GHz, 4 Core(s), 8 Logical Processors
8 GB RAM
Docker Toolbox v18.09.3

Tests took 37 minutes.

image

I tried with both the default VM configuration (1GB RAM, 1 CPU) and an upgraded VM with 4GB RAM and 2 CPU but it didn't have any effect, the tests still took about 37 minutes. It's probably limited by the virtualization technology; the tests cannot utilize the more resources:

image

Test output

On all machines, the console output was this:

$ npm run tests

> versionpress-dev-workspace@1.0.0 tests /Users/borekb/dev/VersionPress/versionpress
> node -r ./scripts/node_modules/ts-node/register scripts/run-tests.ts


> Cleaning up Docker containers and volumes...
Removing network versionpress_default
Network versionpress_default not found.
Removing volume versionpress_db-data-for-tests
Removing volume versionpress_wpcli-cache
Removing volume versionpress_wordpress-files
Removing volume versionpress_test-logs

> Starting MySQL...
Creating network "versionpress_default" with the default driver
Creating volume "versionpress_db-data-for-tests" with default driver
Creating volume "versionpress_wpcli-cache" with default driver
Creating volume "versionpress_wordpress-files" with default driver
Creating volume "versionpress_test-logs" with default driver
Creating versionpress_mysql-for-tests_1 ... done
Waiting for mysql-for-tests:3306  .........  up!
Everything is up

> Starting WordPress...
Creating versionpress_wordpress-for-tests_1 ... done
Waiting for wordpress-for-tests:80  ..  up!
Everything is up

> Running tests...
PHPUnit 5.7.20 by Sebastian Bergmann and contributors.

Runtime:       PHP 7.2.16 with Xdebug 2.6.0
Configuration: /opt/versionpress/tests/phpunit.xml

...............................................................  63 / 738 (  8%)
............................................................... 126 / 738 ( 17%)
............................................................... 189 / 738 ( 25%)
............................................................... 252 / 738 ( 34%)
............................................................... 315 / 738 ( 42%)
............................................................... 378 / 738 ( 51%)
............................................................... 441 / 738 ( 59%)
............................................................... 504 / 738 ( 68%)
............................................................... 567 / 738 ( 76%)
............................................................... 630 / 738 ( 85%)
............S....SS....S..S....S................S....S.S.SSS... 693 / 738 ( 93%)
........S....................S.....FSS......Php Execution Time: 0.032222986221313 Sec
.                   738 / 738 (100%)Bash Execution Time: 0.035927057266235 Sec


Time: 11.13 minutes, Memory: 18.00MB

There was 1 failure:

1) VersionPress\Tests\Workflow\CloneMergeTest::cloneLooksExactlySameAsOriginal
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-		Posted on November 11, 2011Test post for revert
+		Posted on November 11, 2011March 24, 2019Test post
@@ @@

(...see full output below; the diff is several hundreds lines long...)

/opt/versionpress/tests/Workflow/CloneMergeTest.php:210
/opt/versionpress/tests/Workflow/CloneMergeTest.php:48

--

There were 16 skipped tests:

1) VersionPress\Tests\End2End\Media\MediaTest::editationOfFileCreatesCommit
File cannot be changed using WP-CLI.

/opt/versionpress/tests/End2End/Media/MediaTestWpCliWorker.php:48
/opt/versionpress/tests/End2End/Media/MediaTest.php:89

2) VersionPress\Tests\End2End\Menus\MenusTest::creatingMenuItemDraftLeavesNotCleanWorkingDirectory
There is no way to create menu item draft in the WP-CLI

/opt/versionpress/tests/End2End/Menus/MenusWpCliWorker.php:57
/opt/versionpress/tests/End2End/Menus/MenusTest.php:101

3) VersionPress\Tests\End2End\Menus\MenusTest::orphanedMenuItemShouldBeDeletedWhenDoingUndo
This test depends on "VersionPress\Tests\End2End\Menus\MenusTest::creatingMenuItemDraftLeavesNotCleanWorkingDirectory" to pass.

4) VersionPress\Tests\End2End\Options\OptionsTest::changingMoreOptionsCreatesOptionEditAction
More options cannot be changed at once using WP-CLI

/opt/versionpress/tests/End2End/Options/OptionsTestWpCliWorker.php:26
/opt/versionpress/tests/End2End/Options/OptionsTest.php:39

5) VersionPress\Tests\End2End\Pages\PagesTest::updatingPageViaQuickEditWorksEquallyWell
There is nothing like quick edit in the WP-CLI

/opt/versionpress/tests/End2End/Utils/PostTypeTestWpCliWorker.php:51
/opt/versionpress/tests/End2End/Utils/PostTypeTestCase.php:55
/opt/versionpress/tests/End2End/Pages/PagesTest.php:38

6) VersionPress\Tests\End2End\Pages\PagesTest::previewingDraftDoesNotCreateCommit
There is nothing like preview in the WP-CLI

/opt/versionpress/tests/End2End/Utils/PostTypeTestWpCliWorker.php:100
/opt/versionpress/tests/End2End/Utils/PostTypeTestCase.php:127
/opt/versionpress/tests/End2End/Pages/PagesTest.php:93

7) VersionPress\Tests\End2End\Posts\PostsTest::updatingPostViaQuickEditWorksEquallyWell
There is nothing like quick edit in the WP-CLI

/opt/versionpress/tests/End2End/Utils/PostTypeTestWpCliWorker.php:51
/opt/versionpress/tests/End2End/Utils/PostTypeTestCase.php:55
/opt/versionpress/tests/End2End/Posts/PostsTest.php:39

8) VersionPress\Tests\End2End\Posts\PostsTest::previewingDraftDoesNotCreateCommit
There is nothing like preview in the WP-CLI

/opt/versionpress/tests/End2End/Utils/PostTypeTestWpCliWorker.php:100
/opt/versionpress/tests/End2End/Utils/PostTypeTestCase.php:127
/opt/versionpress/tests/End2End/Posts/PostsTest.php:94

9) VersionPress\Tests\End2End\Posts\PostsTest::previewingUnsavedPostCreatesDraft
There is nothing like preview in the WP-CLI

/opt/versionpress/tests/End2End/Utils/PostTypeTestWpCliWorker.php:128
/opt/versionpress/tests/End2End/Utils/PostTypeTestCase.php:145
/opt/versionpress/tests/End2End/Posts/PostsTest.php:115

10) VersionPress\Tests\End2End\Posts\PostsTest::settingFeaturedImageToUnsavedPostDoesNotCommit
Featured image cannot be assigned to unexisting post using WP-CLI

/opt/versionpress/tests/End2End/Utils/PostTypeTestWpCliWorker.php:137
/opt/versionpress/tests/End2End/Utils/PostTypeTestCase.php:175
/opt/versionpress/tests/End2End/Posts/PostsTest.php:144

11) VersionPress\Tests\End2End\Posts\PostsTest::turningUnsavedPostWithFeaturedImageIntoDraftSavesTheFeaturedImage
This test depends on "VersionPress\Tests\End2End\Posts\PostsTest::settingFeaturedImageToUnsavedPostDoesNotCommit" to pass.

12) VersionPress\Tests\End2End\Posts\PostsTest::changingPostFormatUpdatesItsTaxonomy
Post format cannot be changed using WP-CLI.

/opt/versionpress/tests/End2End/Posts/PostsTestWpCliWorker.php:49
/opt/versionpress/tests/End2End/Posts/PostsTest.php:164

13) VersionPress\Tests\End2End\Revert\RevertTest::clickingOnCancelOnlyHidesThePopup
There is no cancel button in the WP-CLI

/opt/versionpress/tests/End2End/Revert/RevertTestWpCliWorker.php:96
/opt/versionpress/tests/End2End/Revert/RevertTest.php:117

14) VersionPress\Tests\End2End\Users\UsersTest::editingMultipleUsermetaCreatesBulkAction
There is no way to change multiple usermeta using WP-CLI

/opt/versionpress/tests/End2End/Users/UsersTestWpCliWorker.php:139
/opt/versionpress/tests/End2End/Users/UsersTest.php:154

15) VersionPress\Tests\Workflow\CloneMergeTest::updatedCloneCanBeMergedBack
This test depends on "VersionPress\Tests\Workflow\CloneMergeTest::cloneLooksExactlySameAsOriginal" to pass.

16) VersionPress\Tests\Workflow\CloneMergeTest::updatedSiteCanBePushedToClone
This test depends on "VersionPress\Tests\Workflow\CloneMergeTest::cloneLooksExactlySameAsOriginal" to pass.

FAILURES!
Tests: 738, Assertions: 950, Failures: 1, Skipped: 16.

> Stopping containers...
Stopping versionpress_wordpress-for-tests_1 ... done
Stopping versionpress_mysql-for-tests_1     ... done
Removing versionpress_wordpress-for-tests_1 ... done
Removing versionpress_mysql-for-tests_1     ... done
Removing network versionpress_default

Note: the above is omits the failing test with many empty lines, see #1400 (comment).

@borekb borekb changed the title Make test runs more reliable New `run-tests.ts`, making test runs more reliable Mar 26, 2019

@borekb borekb merged commit 74ff418 into master Mar 26, 2019

4 of 7 checks passed

Header rules - vp-docs No header rules processed
Details
Pages changed - vp-docs 3 new files uploaded
Details
Redirect rules - vp-docs No redirect rules processed
Details
Mixed content - vp-docs No mixed content detected
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
default Check CodeFlow analysis results.
Details
netlify/vp-docs/deploy-preview Deploy preview ready!
Details

@borekb borekb added this to Done in 4.0-beta → 4.0-beta2 via automation Mar 26, 2019

@borekb borekb deleted the 1400-more-reliable-test-runs branch Mar 26, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.