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

Allow reading bundle from stdin #269

Merged
merged 6 commits into from Sep 17, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 18 additions & 0 deletions cli/README.md
Expand Up @@ -64,3 +64,21 @@ Files in the given directory will be served as static assets.

Options to pass to the server in case `--serve` or `--esm` is being used.
Currently only `--server-option.port` for passing the port to use is supported.

## Spec

The `spec` argument can be a list of files or a glob pattern that will be resolved by Mochify.

```
mochify ./src/foo.test.js ./src/bar.test.js
mochify ./src/*.test.js # Let the shell handle glob expansion
mochify "./src/*.test.js" # Let Mochify handle glob expansion
Comment on lines +74 to +75
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

```

### Reading a bundle from `stdin`

When given `-` as the spec, Mochify expects to read a bundled test suite from `stdin`:

```
browserify -t babelify ./src/*.test.js | mochify -
```
27 changes: 17 additions & 10 deletions cli/index.js
Expand Up @@ -72,6 +72,10 @@ const opts = yargs(hideBin(process.argv))
'$0 --config mochify.webdriver.js "./src/**/*.test.js" ',
'Run all tests matching the given spec using the configuration from mochify.webdriver.js.'
)
.example(
'browserify "./src/**/*.test.js" | $0 -',
'Read a bundled test suite from stdin.'
)
.epilogue(
`Mochify Resources:
GitHub: https://github.com/mantoni/mochify.js`
Expand All @@ -86,16 +90,19 @@ if (opts['server-option']) {
opts.server_options = opts['server-option'];
}

(async () => {
if (opts._.length) {
if (opts._.length) {
if (opts._[0] === '-') {
opts.spec = process.stdin;
} else {
opts.spec = opts._;
}
delete opts._;
try {
const { exit_code } = await mochify(opts);
}
delete opts._;
mochify(opts)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unwrapping this and using a Promise chain is here to stop eslint from complaining. I'm not entirely sure if its complaints are correct or not yet. We could also keep the old version and ignore the rule, no real preference from my side here.

See eslint/eslint#15076

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably even easier to read than the self-invoking async function. 👍

.catch((err) => {
console.error(err.stack);
return { exit_code: 1 };
})
.then(({ exit_code }) => {
process.exitCode = exit_code;
} catch (e) {
console.error(e.stack);
process.exitCode = 1;
}
})();
});
22 changes: 22 additions & 0 deletions cli/test/jsdom.integration.js
@@ -1,5 +1,6 @@
'use strict';

const fs = require('fs');
const path = require('path');
const { assert } = require('@sinonjs/referee-sinon');
const execa = require('execa');
Expand Down Expand Up @@ -28,6 +29,27 @@ describe('jsdom', () => {
assert.equals(json.tests[0].fullTitle, 'test passes');
});

it('reads from stdin', async () => {
let result;
try {
const cp = execa('../../index.js', ['--driver', 'jsdom', '-'], {
cwd: path.join(__dirname, 'fixture')
});
const fixture = fs.createReadStream(
path.resolve(__dirname, './fixture/passes.js')
);
fixture.pipe(cp.stdin);
result = await cp;
} catch (err) {
result = err;
}

assert.isFalse(result.failed);
const json = JSON.parse(result.stdout);
assert.equals(json.tests.length, 1);
assert.equals(json.tests[0].fullTitle, 'test passes');
});

it('fails', async () => {
const result = await run('fails.js');

Expand Down
26 changes: 26 additions & 0 deletions cli/test/playwright.integration.js
@@ -1,5 +1,6 @@
'use strict';

const fs = require('fs');
const path = require('path');
const { assert } = require('@sinonjs/referee-sinon');
const execa = require('execa');
Expand Down Expand Up @@ -35,6 +36,31 @@ describe('playwright', () => {
assert.equals(json.tests[0].fullTitle, 'test passes');
});

it('reads from stdin', async () => {
let result;
try {
const cp = execa(
'../../index.js',
['--driver', 'playwright', '--driver-option.engine', 'firefox', '-'],
{
cwd: path.join(__dirname, 'fixture')
}
);
const fixture = fs.createReadStream(
path.resolve(__dirname, './fixture/passes.js')
);
fixture.pipe(cp.stdin);
result = await cp;
} catch (err) {
result = err;
}

assert.isFalse(result.failed);
const json = JSON.parse(result.stdout);
assert.equals(json.tests.length, 1);
assert.equals(json.tests[0].fullTitle, 'test passes');
});

it('fails', async () => {
const result = await run('fails.js');

Expand Down
22 changes: 22 additions & 0 deletions cli/test/puppeteer.integration.js
@@ -1,5 +1,6 @@
'use strict';

const fs = require('fs');
const path = require('path');
const { assert } = require('@sinonjs/referee-sinon');
const execa = require('execa');
Expand Down Expand Up @@ -28,6 +29,27 @@ describe('puppeteer', () => {
assert.equals(json.tests[0].fullTitle, 'test passes');
});

it('reads from stdin', async () => {
let result;
try {
const cp = execa('../../index.js', ['--driver', 'puppeteer', '-'], {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively we could also teach run about this behaviot, dunno if it's worth it?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That'd make it rather complicated, no?

cwd: path.join(__dirname, 'fixture')
});
const fixture = fs.createReadStream(
path.resolve(__dirname, './fixture/passes.js')
);
fixture.pipe(cp.stdin);
result = await cp;
} catch (err) {
result = err;
}

assert.isFalse(result.failed);
const json = JSON.parse(result.stdout);
assert.equals(json.tests.length, 1);
assert.equals(json.tests[0].fullTitle, 'test passes');
});

it('fails', async () => {
const result = await run('fails.js');

Expand Down
18 changes: 9 additions & 9 deletions mochify/index.js
Expand Up @@ -2,6 +2,7 @@

const { readFile } = require('fs').promises;
const { loadConfig } = require('./lib/load-config');
const { validateConfig } = require('./lib/validate-config');
const { setupClient } = require('./lib/setup-client');
const { createMochaRunner } = require('./lib/mocha-runner');
const { resolveBundle } = require('./lib/resolve-bundle');
Expand All @@ -14,11 +15,16 @@ exports.mochify = mochify;
async function mochify(options = {}) {
const config = await loadConfig(options);

const validation_error = validateConfig(config);
if (validation_error) {
throw validation_error;
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is validateConfig not throwing the errors directly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like I've just been writing too much Go lately. I'll change this as it makes sense for this to behave like everything else (which is throwing to stop execution).


// Create runner early to verify the reporter exists:
const mocha_runner = createMochaRunner(config.reporter || 'spec');
const { mochifyDriver } = resolveMochifyDriver(config.driver);

const [mocha, client, files] = await Promise.all([
const [mocha, client, resolved_spec] = await Promise.all([
readFile(require.resolve('mocha/mocha.js'), 'utf8'),
readFile(require.resolve('./client'), 'utf8'),
resolveSpec(config.spec)
Expand All @@ -30,7 +36,7 @@ async function mochify(options = {}) {
let server = null;
if (config.serve || config.esm) {
const _scripts = [mocha, configured_client];
const _modules = config.esm ? files : [];
const _modules = config.esm ? resolved_spec : [];
server = await startServer(
config.serve || process.cwd(),
Object.assign({ _scripts, _modules }, config.server_options)
Expand All @@ -41,7 +47,7 @@ async function mochify(options = {}) {
const driver_promise = mochifyDriver(driver_options);
const bundler_promise = config.esm
? Promise.resolve('')
: resolveBundle(config.bundle, files);
: resolveBundle(config.bundle, resolved_spec);

let driver, bundle;
try {
Expand Down Expand Up @@ -72,12 +78,6 @@ async function mochify(options = {}) {
}

function resolveMochifyDriver(name) {
if (!name) {
throw new Error(
'Specifying a driver option is required. Mochify drivers need to be installed separately from the API or the CLI.'
);
}

let driverModule;
try {
// eslint-disable-next-line node/global-require
Expand Down
4 changes: 3 additions & 1 deletion mochify/lib/load-config.js
Expand Up @@ -22,7 +22,9 @@ async function loadConfig(options) {
async function mergeWithDefault(default_config_promise, config) {
const default_config = await default_config_promise;
if (default_config) {
return deepmerge(default_config.config, config);
return deepmerge(default_config.config, config, {
clone: false
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cloning a stream apparently causes infinite recursion. Don't think not cloning will hurt us here.

});
}
return config;
}
22 changes: 19 additions & 3 deletions mochify/lib/resolve-bundle.js
Expand Up @@ -6,14 +6,21 @@ const { parseArgsStringToArgv } = require('string-argv');

exports.resolveBundle = resolveBundle;

async function resolveBundle(command, files) {
async function resolveBundle(command, resolved_spec) {
if (
typeof resolved_spec === 'object' &&
typeof resolved_spec.pipe === 'function'
) {
return bufferStream(resolved_spec);
}

if (!command) {
return concatFiles(files);
return concatFiles(resolved_spec);
}

const [cmd, ...args] = parseArgsStringToArgv(command);

const result = await execa(cmd, args.concat(files), {
const result = await execa(cmd, args.concat(resolved_spec), {
preferLocal: true
});

Expand All @@ -28,3 +35,12 @@ async function concatFiles(files) {
const buffers = await Promise.all(files.map((file) => fs.readFile(file)));
return Buffer.concat(buffers).toString('utf8');
}

function bufferStream(stream) {
return new Promise((resolve, reject) => {
const buffers = [];
stream.on('data', (chunk) => buffers.push(chunk));
stream.on('error', (err) => reject(err));
stream.on('end', () => resolve(Buffer.concat(buffers).toString('utf8')));
});
}
4 changes: 4 additions & 0 deletions mochify/lib/resolve-spec.js
Expand Up @@ -6,6 +6,10 @@ const glob = promisify(require('glob'));
exports.resolveSpec = resolveSpec;

async function resolveSpec(spec = 'test/**/*.js') {
if (typeof spec === 'object' && typeof spec.pipe === 'function') {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found it nicer to handle such situations at lib level instead of index.js as index.js already contains a lot of control flow and it would have gotten even more.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I also get the feeling it's growing quite a bit.

Maybe it's time to introduce more functions like run that handle parts of the control flow. E.g. configure, launch, run and shutdown. Just a random morning thought though 🙃

return spec;
}

const patterns = Array.isArray(spec) ? spec : [spec];
const matches = await Promise.all(patterns.map((pattern) => glob(pattern)));
return matches.reduce((all, match) => all.concat(match), []);
Expand Down
7 changes: 7 additions & 0 deletions mochify/lib/resolve-spec.test.js
@@ -1,5 +1,6 @@
'use strict';

const fs = require('fs');
const proxyquire = require('proxyquire');
const { assert, sinon } = require('@sinonjs/referee-sinon');

Expand Down Expand Up @@ -75,4 +76,10 @@ describe('mochify/lib/resolve-spec', () => {

await assert.rejects(promise, error);
});

it('passes through streams', async () => {
const stream = fs.createReadStream(__filename);
const promise = resolveSpec(stream);
await assert.resolves(promise, stream);
});
});
22 changes: 22 additions & 0 deletions mochify/lib/validate-config.js
@@ -0,0 +1,22 @@
'use strict';

exports.validateConfig = validateConfig;

function validateConfig(config) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left this function intentionally simple (i.e. manual checking) for now. We can start building our declarative mini DSL later ...

if (!config.driver) {
return new Error(
'Specifying a `driver` is required. Mochify drivers need to be installed separately from the API or the CLI.'
);
}
if (config.esm && config.bundle) {
return new Error('`esm` cannot be used in conjunction with `bundle`');
}
if (
config.bundle &&
typeof config.spec === 'object' &&
typeof config.spec.pipe === 'function'
) {
return new Error('`bundle` cannot be used when `spec` is a stream.');
}
return null;
}
42 changes: 42 additions & 0 deletions mochify/lib/validate-config.test.js
@@ -0,0 +1,42 @@
'use strict';

const fs = require('fs');
const { assert } = require('@sinonjs/referee-sinon');
const { validateConfig } = require('./validate-config');

describe('mochify/lib/validate-config', () => {
it('returns an error when esm and bundle are given', () => {
assert.isError(
validateConfig({
driver: 'puppeteer',
esm: true,
bundle: 'browserify',
spec: './test.js'
})
);
});

it('returns an error when bundle and a stream spec are given', () => {
assert.isError(
validateConfig({
driver: 'puppeteer',
bundle: 'browserify',
spec: fs.createReadStream(__filename)
})
);
});

it('returns an error on an empty config', () => {
assert.isError(validateConfig({}));
});

it('returns null on a valid config', () => {
assert.isNull(
validateConfig({
bundle: 'browserify -t babelify',
spec: './test.js',
driver: 'puppeteer'
})
);
});
});