[PP-1] Ensure availability of node/npm in the testrunners

That should be it

Updating issue to reflect #9.

Also commented at #2609560-33: Base DCI containers off official containers, and bumped that to major, since it blocks this major issue.

This blocks #2575501: [PP-2] DrupalCI should return failed for poor code style - JS, so tagging.

+++ b/containers/base/php-base/Dockerfile
@@ -63,8 +65,9 @@ RUN apt-get clean && apt-get update && \
+    npm install -g eslint

Just a couple of issues. Installing eslint is probably not what we want to do, this will keep the version of eslint locked at the time of the Docker image creation, any additional improvements we will miss out on. Leaving the install of eslint to the package.json that should be included with core will help.

#13: but that would mean the environment would need to be updated for every test run. Because the same test runner must be used for Drupal 8.1, 8.2, 8.3 …

So I think it's more feasible to specify a particular version of eslint, and then frequently update it: whenever core updates it. Or, alternatively, use whatever the latest eslint is. Or does it change frequently in non-BC ways?

There is a new release 2-3 times a months these days. Fixing bugs, not introducing BC issues (it follows semver).

Don't we download the PHP dependencies with composer each time? This feels mostly the same.

composer fetches code that is part of the Drupal application. It includes code that is executed.

eslint is neither.

Don't we download the PHP dependencies with composer each time? This feels mostly the same.

It is the same.

For instance, all Drupal PHP CS issues are against coder 8.2.8 right now: #2744463: Add phpcs, coder 8.2.8 as --dev requirements for drupal/core Once that patch is in, both the testbot and all developers will end up installing and using the same version of the tool so we can have consistency.

The same should be true of eslint. Is there an issue in core for adding a package.json file? Once that's present we can do npm install.

As far as the environment present on the testbot, we'd need to specify npm in order to manage the package. HOWEVER. :-) Testbot is still undergoing some architectural shifts and we're not sure exactly where this functionality will land. So we can't just add it right now.

#2809165: Add package.json to Drupal

Core now has core/package.json after #2809477: Add ES6 to ES5 build process

Unless I'm completely mistaken, that means we should use npm to build out core's JS dependencies during the build process. That will give us eslint which we can then execute from a code_assessment build step. That will see core's .eslintrc.json file and do an assessment.

So the test environment needs node/npm, so we can make an npm plugin and an eslint plugin.

The design philosophy of drupalci has altered somewhat in that we are now focusing on having a defined host environment for running a testbot.

What this means is that we do not need to rely on docker containers in order to add codebase validation or building functionality to the testbots, as we can rely on that software being installed on the host.

As such, I've added npm/node to the host system and it now exists in both the AMI as well as in the vagrant box. eslint will update whenever we update the hostAMI or the packer build (it will not be installed as part of the build process) as such, drpal's concerns about eslint getting behind are valid, however its probably stable enough that we do not need to be on the bleeding edge of it, and its very easy to update the version/ami at will.

This is no longer blocking eslint/csslint, and work is already proceeding on those, so Im marking this fixed.

@Mixologic

Could you clarify which version of Node, eslint are installed currently?

Sure thing:

testbot@drupalci:~$ node -v
v6.9.5
testbot@drupalci:~$ eslint -v
v3.15.0
testbot@drupalci:~$