Propagate process exit code in integration test script

[sudo-suhas]

I noticed that even though the integration test failed, the pre-commit hook did not fail. The status code returned by the child process was not being propagated. So this PR does the following:

• If any tests(right now only 'basic') fail, the script exits with exit code 1.
• Adds some logging.
• Makes the script and helpers node 4 compatible. Since we wouldn't be developing on node 4, testing it on CI is a good idea.
• Tweak travis config:
  • Test on node 4, 9.
  • Stop testing on node 7.

[sudo-suhas]

Tests are still failing on node 4 because it is unable to find some modules. This happens because npm@2 does not flatten installed modules and node is not able to find some packages which would be nested deeply in the project root's node_modules.

We could do any of the following to fix the issue:

• As part of the install step on travis, also install modules in integration test fixtures.
• Run npm install before npm test in test script.
• Install a newer version of npm on node 4 so that node_modules are flattened. 🡠 I like this one

Edit: I went with the third option.

[asfktz]

We could do any of the following to fix the issue:

• As part of the install step on travis, also install modules in integration test fixtures.
• Run npm install before npm test in test script.
• Install a newer version of npm on node 4 so that node_modules are flattened. 🡠 I like this one

What are the downsides of the first option?

[sudo-suhas]

The installation step would need to cd into each folder in specs/fixtures and run install. The CI config would need to be updated each time we add an integration test. However, the current approach might fail if the integration test has a dependency not installed in the project root.

[asfktz]

Hmmm.. if I understands correctly, sound like it can be automated by adding the installing step to ./scripts/runIntegrationTests.js:

autodll-webpack-plugin/scripts/runIntegrationTests.js

Lines 1 to 18 in 78ca765

	const { spawnSync } = require('child_process');
	const { join } = require('path');
	const { readdirSync, lstatSync } = require('fs');
	const cwd = join(__dirname, '../specs/fixtures');
	const isDirectory = source => lstatSync(join(cwd, source)).isDirectory();
	readdirSync(cwd)
	.filter(isDirectory)
	.map(dirname => join(cwd, dirname))
	.forEach(ctx => {
	spawnSync('npm', ['test'], {
	cwd: ctx,
	shell: true,
	stdio: 'inherit',
	});
	});

Something like:

readdirSync(cwd)
  .filter(isDirectory)
  .map(dirname => join(cwd, dirname))
  .forEach(ctx => {
    spawnSync('npm', ['install'], {
      cwd: ctx,
      shell: true,
      stdio: 'inherit',
    });
  })
  .forEach(ctx => {
    spawnSync('npm', ['test'], {
      cwd: ctx,
      shell: true,
      stdio: 'inherit',
    });
  });

Will it do the trick?

[sudo-suhas]

Yea, that was option 2. I'll update the PR to that instead.

[asfktz]

Ohh sorry, I missed that

[sudo-suhas]

No problem 😃

[sudo-suhas]

This is why the tests are still failing on node 4:

λ cd specs\fixtures\basic\

λ node
> require.resolve('write-pkg')
'E:\\Projects\\repos\\autodll-webpack-plugin\\specs\\fixtures\\basic\\node_modules\\write-pkg\\index.js'
> .exit

λ cd ..\..\helpers\integration\

λ node
> require.resolve('write-pkg')
'E:\\Projects\\repos\\autodll-webpack-plugin\\node_modules\\write-pkg\\index.js'

The package resolution for integration tests helpers will be up the directory tree which does not include specs/fixtures/basic. While the installation step is still useful, our issue is not yet resolved. That leaves us with 2 options:

• Move the helpers inside specs/fixtures/basic. This would mean that we won't be able to share them between integration tests, if and when we have multiple.
• Use npm@3 on node v4. This approach isn't guaranteed to succeed as it is somewhat a coincidence that write-pkg is a transient dependency:

  λ yarn why write-pkg
  yarn why v1.3.2
  [1/4] Why do we have the module "write-pkg"...?
  [2/4] Initialising dependency graph...
  [3/4] Finding dependency...
  [4/4] Calculating file sizes...
  info This module exists because "ava#ava-init" depends on it.
  Done in 1.33s.
  

• Add the packages required by integration test helpers to our dev-deps. 🡠 I like this one

[sudo-suhas]

I went with the third option.

I have added the installation step as a separate script. The rationale for this is that in our development environment, if we want to run integration tests, we shouldn't have to run npm install(it is particularly buggy on windows). I'd much rather manage that manually. Plus, we can simply do npm run install:fixturedeps if needed.

[asfktz]

I see.
Let's go with that 👍