Problem/Motivation

For running tests the testbot right now basically does:

/core/scripts/run-tests.sh --concurrency=31
/core/scripts/run-tests.sh --types='PHPUnit-FunctionalJavascript' --concurrency=1

On top of that we need to run something like
yarn test

Proposed resolution

  • Somehow hack into run-tests.sh to also run yarn test
  • Expand the testbot to run yarn test directly.
    Therefore we would need a new testing "build step". Therefore in https://www.drupal.org/project/drupalci_testbot clone somehow copy
    src/DrupalCI/Plugin/BuildTask/BuildStep/Testing/Simpletest.php and run something in ::run

    Finally we would need to expand the list in build_definitions/simpletest.yml

Remaining tasks

User interface changes

API changes

Data model changes

Comments

dawehner created an issue. See original summary.

dawehner’s picture

Issue summary: View changes
Mixologic’s picture

I would vastly prefer the latter. run-tests.sh provides some key functionality, but its a burning tire fire when it comes to maintainability.

I dont know enough about yarn test to know if we need some sort of wrapper or parallelization/isolation, i.e. what happens if a test fatally crashes (which, I think would be patches with invalid javascript and the like)

Im tempted to say lets start without one, just run yarn test on the testbots and see how far that gets us until we hit more issues that need to be worked around.

dawehner’s picture

Well: yarn test is just an alias for yarn run test which executes whatever is in the package.json file.

I don't know much about error handling in nightwatch, but there seems to be at least some issues discussing some problems out there:

I hope @justafish can be some more insight into high scale nightwatch usages.

dawehner’s picture

Title: Run javascript tests from within run-tests.sh » Run ```yarn test``` on the testbot
Project: Drupal core » DrupalCI: Test Runner
Version: 8.5.x-dev »
Component: simpletest.module » Documentation

Our latest agreement is to run ```yarn test``` on the testbot.

Mixologic’s picture

Title: Run ```yarn test``` on the testbot » Run yarn test on the testbot
Mixologic’s picture

Status: Active » Needs work

Allrighty, I have yarn and nodejs installed in all the containers, deployed to production.

So this is unblocked, and I'll proceed to work on the javascript testing function. I think I might call it nightwatch specific vs just 'javascript' as I think there's going to be a lot of configuration stuff thats going to be specific to nightwatch.

  • 1574c3a committed on 2928962-javascript-testing
    Issue #2928962: Adds the javascript build step. Needs much more work
    
  • 697a6fe committed on 2928962-javascript-testing
    Issue #2928962: fixes a accident and a typo
    
Mixologic’s picture

Testbot now has yarn in the docker containers, as well as on the host.

dawehner’s picture

🎆

Mile23’s picture

Making a yarn build step here: #2874028: Create a Yarn Build step

The build step just says yarn install.

If we're going to run tests via yarn, we should make a yarn plugin that takes commands, so we can just say yarn:option:"run test" in the build file. We might also use yarn for css and js linting, because it's right there in the package.json.

If we're not going to use those commands, then there should be a core issue to remove them or document appropriate use.

Really, what we need is a core issue about managing all the JS/CSS build stuff so there's a command to ecma-fy all JS and CSS-ify all SASS and so forth, for all of core. But that's out of scope here, of course.

Mixologic’s picture

yarn test acts as a convenient go between for running nightwatch and having a convenient commandline tool to do that.

As such we can kindof invert the typical drupalci pattern to be more flexible for developers: I..e. we should be doing things like setting environment variables that will ultimately get consumbed by the test runner as config, vs executing the test runner (in this case nightwatch) directly.

That way, if for whatever reason somebody wants to submit a patch that changes how yarn test behaves, its possible for them to do that. IOW in a perfect world, drupalci communicates all the necessary metadata to the system under test via environment variables - where to put artifacts, what tests to run, how to connect to the database, what urls to use to reach the SUT etc.

Given that what we're doing here is running tests, and not say, running gulp tasks, or assembling the codebase, or any of the other yarn, Im inclined to avoid having a plugin that can run `yarn $whatever` - I think its valuable to have the plugins be separate things one for yarn_install, and one for yarn_test. Maybe there's an abstract plugin/trait for yarn that shares code between the two, but mostly yarn test:js is just a hand off for nightwatch, so yarn_test is really the nightwatch plugin.

So, yarn eslint or yarn sass or whatever each of those commands do, probs should be small plugins that focus on the underlying purpose of the command, vs the mechanism that executes the command (yarn), Otherwise the yarn plugin couldn end up getting packed with all the necessary configuration options and logic to handle *everything that yarn can do* which has the potential to be unwieldy and or dense. (similar to how we have both the composer plugin, and the container_composer plugin - same underlying tool, different purposes)

  • 42a1f1e committed on 2928962-javascript-testing
    Issue #2928962: protects against failure if there is no nightwatch in...

  • 2e83b54 committed on 2928962-javascript-testing
    Issue #2928962: if links are alreddy done, dont bail out hard
    
Mile23’s picture

Title: Run yarn test on the testbot » Run yarn-based core tests on the testbot

So, yarn eslint or yarn sass or whatever each of those commands do, probs should be small plugins that focus on the underlying purpose of the command, vs the mechanism that executes the command (yarn)

Totally right.

+1 on calling it nightwatch_js.

Do we just not have any use cases to test again?

Mixologic’s picture

oh, sorry, I've been talking to dawener/drpal/justafish at cons and in #javascript/#phpunit in drupalslack, and there's a fork of drupal with all the nightwatch stuff built in it on github here, along with a pullrequest that works: https://github.com/jsdrupal/drupal/pull/5

I ran it on dispatcher using the 'DrupalCI Job tester" to see if I could get it going, and I think we're really close: https://dispatcher.drupalci.org/job/drupalci_test_containers/929/console

It doesnt work with sqlite yet, so I dont think I can flip it on entirely, and a test fails with whatever new node dependencies somehow get caught up in the Yaml testing (some stray travis.yml file in a node_modules directory in this case) We might need to open a core issue to make that test not so "scour the filesystem looking for yaml" or something.

This one is running right now, and should have the added benefit of jenkins parsing the additional test suite and xml files: https://dispatcher.drupalci.org/job/drupalci_test_containers/930/

Mixologic’s picture

Also, *this* plugin should definitely still work with contrib as well, allowing them to do nightwatch tests also. But I think we got to get core off the ground first and then extend core's testing platform to contrib.

  • 9af5473 committed on 2928962-javascript-testing
    Issue #2928962: runs nightwatch tests as www-data
    
Mixologic’s picture

Status: Needs work » Needs review

Okay, I went ahead and merged this into dev, and its testing now. I have confirmed that we can make this work for contrib by merely passing in a subdirectory where contrib keeps its nightwatch tests.

There are a few things that nightwatch allows one to do that contrib wont be able to do right off the bat, but I believe should be solveable by having drupalci edit the nightwatch.conf.js file.

All three of the options, custom_commands_path, custom_assertions_path, page_objects_path, can be comma separated lists, so if its a contrib test, we would need to edit that file to allow them to extend nightwatch if they needed to.

I didnt add any tests to this plugin yet because theres not really a stable nightwatch implementation in core yet to prove that it works with.

Theres other improvements that can happen too, like right now there's some ugly errors like 'error Command "test:js" not found.' -> we'll probably want that to check whether or not there is a config file as to whether or not to run the tests or not, as we'll want a mechanism to skip testing if contrib doesnt have any. (or, like now, since core doesnt either.)

  • 1574c3a committed on 2928962-add-test
    Issue #2928962: Adds the javascript build step. Needs much more work
    
  • 2e83b54 committed on 2928962-add-test
    Issue #2928962: if links are alreddy done, dont bail out hard
    
  • 42a1f1e committed on 2928962-add-test
    Issue #2928962: protects against failure if there is no nightwatch in...
  • 697a6fe committed on 2928962-add-test
    Issue #2928962: fixes a accident and a typo
    
  • 9af5473 committed on 2928962-add-test
    Issue #2928962: runs nightwatch tests as www-data
    
  • Mile23 committed c51d68f on 2928962-add-test
    Issue #2928962: Add minimal testing for a little insurance. Will add...
Mile23’s picture

2928962-add-test branch adds a minimal test, just so there's something exercising the code at test time.

Mixologic’s picture

Status: Needs review » Fixed

This is deployed and pushed into production branch along with the test from #21, thanks bunches.

We can open new issue when this is in core and nightwatch tests are actually running.

dawehner’s picture

That's vnice: 🎆🎆🎆🎆

Status: Fixed » Closed (fixed)

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