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

Basic support for Jest runner #335

Merged
merged 20 commits into from
Oct 17, 2017
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 44 additions & 16 deletions detox/local-cli/detox-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const program = require('commander');
const path = require('path');
const cp = require('child_process');
program
.option('-r, --runner [runner]', 'Test runner (currently supports mocha)', 'mocha')
.option('-r, --runner [runner]', 'Test runner (currently supports mocha)')
.option('-o, --runner-config [config]', 'Test runner config file', 'mocha.opts')
.option('-l, --loglevel [value]', 'info, debug, verbose, silly, wss')
.option('-c, --configuration [device configuration]', 'Select a device configuration from your defined configurations,'
Expand All @@ -21,26 +21,54 @@ program
const config = require(path.join(process.cwd(), 'package.json')).detox;
const testFolder = config.specs || 'e2e';

const loglevel = program.loglevel ? `--loglevel ${program.loglevel}` : '';
const configuration = program.configuration ? `--configuration ${program.configuration}` : '';
const cleanup = program.cleanup ? `--cleanup` : '';
const reuse = program.reuse ? `--reuse` : '';
const artifactsLocation = program.artifactsLocation ? `--artifacts-location ${program.artifactsLocation}` : '';
let runner = config.runner || 'mocha';

if (typeof program.debugSynchronization === "boolean") {
program.debugSynchronization = 3000;
if (program.runner) {
runner = program.runner;
}
let debugSynchronization = program.debugSynchronization ? `--debug-synchronization ${program.debugSynchronization}` : '';

function runMocha() {
const loglevel = program.loglevel ? `--loglevel ${program.loglevel}` : '';
const configuration = program.configuration ? `--configuration ${program.configuration}` : '';
const cleanup = program.cleanup ? `--cleanup` : '';
const reuse = program.reuse ? `--reuse` : '';
const artifactsLocation = program.artifactsLocation ? `--artifacts-location ${program.artifactsLocation}` : '';

let command;
switch (program.runner) {
if (typeof program.debugSynchronization === "boolean") {
program.debugSynchronization = 3000;
}

const debugSynchronization = program.debugSynchronization ? `--debug-synchronization ${program.debugSynchronization}` : '';
const command = `node_modules/.bin/mocha ${testFolder} --opts ${testFolder}/${program.runnerConfig} ${configuration} ${loglevel} ${cleanup} ${reuse} ${debugSynchronization} ${artifactsLocation}`;

console.log(command);
cp.execSync(command, {stdio: 'inherit'});
}

function runJest() {
const command = `node_modules/.bin/jest ${testFolder} --runInBand`;
console.log(command);
cp.execSync(command, {
stdio: 'inherit',
env: {
...process.env,
configuration: program.configuration,
loglevel: program.loglevel,
cleanup: program.cleanup,
reuse: program.reuse,
debugSynchronization: program.debugSynchronization ? 3000 : '',
Copy link
Member

Choose a reason for hiding this comment

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

@Kureev , wouldn't this override every number value with 3000 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @rotemmiz! Nice catch, fixed! 😉

artifactsLocation: program.artifactsLocation
}
});
}

switch (runner) {
case 'mocha':
command = `node_modules/.bin/${program.runner} ${testFolder} --opts ${testFolder}/${program.runnerConfig} ${configuration} ${loglevel} ${cleanup} ${reuse} ${debugSynchronization} ${artifactsLocation}`;
runMocha();
break;
case 'jest':
Copy link
Member

Choose a reason for hiding this comment

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

What about passing detox command line args? Will those work?

Copy link
Contributor Author

@Kureev Kureev Oct 11, 2017

Choose a reason for hiding this comment

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

We can think of a way doing this. Although, in this version I wasn't aiming to provide this. If this is mandatory, we can discuss the approach

Sorry, misread your comment. What do you mean by "detox cli args"? Can you make an example?

Copy link
Member

Choose a reason for hiding this comment

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

I think it is, the feature is not 100% usable without it.

Copy link
Member

Choose a reason for hiding this comment

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

detox test --configuration ios.sim.release
detox test --loglevel verbose
detox test --debug-synchronization

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which flags are important?
In the previous version there were no recommendations regarding flags so kept it out of scope. However, I'm fine adding important flags to make jest support even better 💪 .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rotemmiz what is the goal?

  • tests can access CLI arguments
    or
  • detox can access CLI arguments

If we talk about the second one, then it already works like this (commander parses arguments before passing them to test runner)

Copy link
Member

Choose a reason for hiding this comment

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

You're right, but we execute a child process and these argument are not being passed into it.
cp.execSync(command, {stdio: 'inherit'});
We probably just need to pass them on...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, now I see. Indeed, we just need to pass arguments to the child process. Then there will be no point in passing all these flags in the command itself

Copy link
Contributor Author

@Kureev Kureev Oct 11, 2017

Choose a reason for hiding this comment

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

Seems like jest doesn't support allow custom CLI parameters. The only approach I see is to use environment vars.

Using ENV vars will imply changes on the mocha part and the way test cases gets parameters overall.

UPD: I can update argparse library and CLI so instead of additional flags it'll create one-time ENV variables. For user it'll be still the same, all mapping will happen under the hood.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice idea @Kureev, thank you so much for your efforts here 👍

runJest();
break;
default:
throw new Error(`${program.runner} is not supported in detox cli tools. You can still run your tests with the runner's own cli tool`);
throw new Error(`${runner} is not supported in detox cli tools. You can still run your tests with the runner's own cli tool`);
}

console.log(command);
cp.execSync(command, {stdio: 'inherit'});
7 changes: 3 additions & 4 deletions detox/src/Detox.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ const DEVICE_CLASSES = {
};

class Detox {

constructor(userConfig) {
if (!userConfig) {
throw new Error(`No configuration was passed to detox, make sure you pass a config when calling 'detox.init(config)'`);
Expand All @@ -33,10 +32,10 @@ class Detox {
this.device = null;
this._currentTestNumber = 0;
const artifactsLocation = argparse.getArgValue('artifacts-location');
if(artifactsLocation !== undefined) {
if (artifactsLocation !== undefined) {
try {
this._artifactsPathsProvider = new ArtifactsPathsProvider(artifactsLocation);
} catch(ex) {
} catch (ex) {
log.warn(ex);
}
}
Expand Down Expand Up @@ -130,7 +129,7 @@ class Detox {
}

if (!deviceConfig) {
throw new Error(`Cannot determine which configuration to use. use --configuration to choose one of the following:
throw new Error(`Cannot determine which configuration to use. use --configuration to choose one of the following:
${Object.keys(configurations)}`);
}

Expand Down
47 changes: 22 additions & 25 deletions detox/src/Detox.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,16 @@ describe('Detox', () => {
let fs;
let Detox;
let detox;
let minimist;
let clientMockData = {lastConstructorArguments: null};
let deviceMockData = {lastConstructorArguments: null};
const clientMockData = {lastConstructorArguments: null};
const deviceMockData = {lastConstructorArguments: null};

beforeEach(async () => {
function setCustomMock(modulePath, dataObject) {
const JestMock = jest.genMockFromModule(modulePath);
class FinalMock extends JestMock {
constructor() {
super(...arguments);
dataObject.lastConstructorArguments = arguments;
constructor(...rest) {
super(rest);
dataObject.lastConstructorArguments = rest;
}
}
jest.setMock(modulePath, FinalMock);
Expand All @@ -23,12 +22,12 @@ describe('Detox', () => {
jest.mock('npmlog');
jest.mock('fs');
fs = require('fs');
jest.mock('minimist');
minimist = require('minimist');
jest.mock('./ios/expect');
setCustomMock('./client/Client', clientMockData);
setCustomMock('./devices/Device', deviceMockData);

process.env = {};

jest.mock('./devices/IosDriver');
jest.mock('./devices/SimulatorDriver');
jest.mock('./devices/Device');
Expand Down Expand Up @@ -66,7 +65,7 @@ describe('Detox', () => {
});

it(`Passing --cleanup should shutdown the currently running device`, async () => {
mockCommandLineArgs({cleanup: true});
process.env.cleanup = true;
Detox = require('./Detox');

detox = new Detox(schemes.validOneDeviceNoSession);
Expand Down Expand Up @@ -99,7 +98,7 @@ describe('Detox', () => {
});

it(`Two valid devices, detox should init with the device passed in '--configuration' cli option`, async () => {
mockCommandLineArgs({configuration: 'ios.sim.debug'});
process.env.configuration = 'ios.sim.debug';
Detox = require('./Detox');

detox = new Detox(schemes.validTwoDevicesNoSession);
Expand All @@ -108,7 +107,7 @@ describe('Detox', () => {
});

it(`Two valid devices, detox should throw if device passed in '--configuration' cli option doesn't exist`, async () => {
mockCommandLineArgs({configuration: 'nonexistent'});
process.env.configuration = 'nonexistent';
Detox = require('./Detox');

detox = new Detox(schemes.validTwoDevicesNoSession);
Expand All @@ -121,7 +120,7 @@ describe('Detox', () => {
});

it(`Two valid devices, detox should throw if device passed in '--configuration' cli option doesn't exist`, async () => {
mockCommandLineArgs({configuration: 'nonexistent'});
process.env.configuration = 'nonexistent';
Detox = require('./Detox');

detox = new Detox(schemes.validTwoDevicesNoSession);
Expand Down Expand Up @@ -165,22 +164,22 @@ describe('Detox', () => {
});

it(`Detox should use session defined per configuration - none`, async () => {
mockCommandLineArgs({configuration: 'ios.sim.none'});
process.env.configuration = 'ios.sim.none';
Detox = require('./Detox');
detox = new Detox(schemes.sessionPerConfiguration);
await detox.init();

let expectedSession = schemes.sessionPerConfiguration.configurations['ios.sim.none'].session;
const expectedSession = schemes.sessionPerConfiguration.configurations['ios.sim.none'].session;
expect(clientMockData.lastConstructorArguments[0]).toBe(expectedSession);
});

it(`Detox should use session defined per configuration - release`, async () => {
mockCommandLineArgs({configuration: 'ios.sim.release'});
process.env.configuration = 'ios.sim.release';
Detox = require('./Detox');
detox = new Detox(schemes.sessionPerConfiguration);
await detox.init();

let expectedSession = schemes.sessionPerConfiguration.configurations['ios.sim.release'].session;
const expectedSession = schemes.sessionPerConfiguration.configurations['ios.sim.release'].session;
expect(clientMockData.lastConstructorArguments[0]).toBe(expectedSession);
});

Expand All @@ -189,12 +188,12 @@ describe('Detox', () => {
detox = new Detox(schemes.sessionInCommonAndInConfiguration);
await detox.init();

let expectedSession = schemes.sessionInCommonAndInConfiguration.configurations['ios.sim.none'].session;
const expectedSession = schemes.sessionInCommonAndInConfiguration.configurations['ios.sim.none'].session;
expect(clientMockData.lastConstructorArguments[0]).toBe(expectedSession);
});

it(`beforeEach() - should set device artifacts destination`, async () => {
mockCommandLineArgs({'artifacts-location': '/tmp'});
process.env.artifactsLocation = '/tmp';
Detox = require('./Detox');
detox = new Detox(schemes.validOneDeviceAndSession);
await detox.init();
Expand All @@ -211,7 +210,7 @@ describe('Detox', () => {
});

it(`afterEach() - should call device.finalizeArtifacts`, async () => {
mockCommandLineArgs({'artifacts-location': '/tmp'});
process.env.artifactsLocation = '/tmp';
Detox = require('./Detox');
detox = new Detox(schemes.validOneDeviceAndSession);
await detox.init();
Expand All @@ -228,13 +227,11 @@ describe('Detox', () => {
});

it(`the constructor should catch exception from ArtifactsPathsProvider`, async () => {
mockCommandLineArgs({'artifacts-location': '/tmp'});
fs.mkdirSync = jest.fn(() => {throw 'Could not create artifacts root dir'});
process.env.artifactsLocation = '/tmp';
fs.mkdirSync = jest.fn(() => {
throw Error('Could not create artifacts root dir');
});
Detox = require('./Detox');
detox = new Detox(schemes.validOneDeviceAndSession);
});

function mockCommandLineArgs(args) {
minimist.mockReturnValue(args);
}
});
10 changes: 9 additions & 1 deletion detox/src/utils/argparse.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
const argv = require('minimist')(process.argv.slice(2));

function getArgValue(key) {
const value = argv ? argv[key] : undefined;
let value;

if (argv && argv[key]) {
value = argv[key];
} else {
const camelCasedKey = key.replace(/(\-\w)/g, (m) => m[1].toUpperCase());
value = process.env[camelCasedKey];
}

return value;
}

Expand Down
43 changes: 31 additions & 12 deletions detox/src/utils/argparse.test.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,39 @@
const _ = require('lodash');
jest.unmock('process');

describe('argparse', () => {
let argparse;
describe('using env variables', () => {
let argparse;

beforeEach(() => {
jest.mock('minimist');
const minimist = require('minimist');
minimist.mockReturnValue({test: 'a value'});
argparse = require('./argparse');
});
beforeEach(() => {
process.env.fooBar = 'a value';
argparse = require('./argparse');
});

it(`nonexistent key should return undefined result`, () => {
expect(argparse.getArgValue('blah')).not.toBeDefined();
});

it(`nonexistent key should return undefined result`, () => {
expect(argparse.getArgValue('blah')).not.toBeDefined();
it(`existing key should return a result`, () => {
expect(argparse.getArgValue('foo-bar')).toBe('a value');
});
});

it(`existing key should return a result`, () => {
expect(argparse.getArgValue('test')).toBe('a value');
describe('using arguments', () => {
let argparse;

beforeEach(() => {
jest.mock('minimist');
const minimist = require('minimist');
minimist.mockReturnValue({'kebab-case-key': 'a value'});
argparse = require('./argparse');
});

it(`nonexistent key should return undefined result`, () => {
expect(argparse.getArgValue('blah')).not.toBeDefined();
});

it(`existing key should return a result`, () => {
expect(argparse.getArgValue('kebab-case-key')).toBe('a value');
});
});
});
9 changes: 4 additions & 5 deletions docs/Guide.Jest.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,9 @@ npm install --save-dev jest

You should remove `e2e/mocha.opts`, you no longer need it.

### 3. Write a detox setup file
### 3. Replace generated detox setup file (e2e/init.js)

```js
// ./jest/setup/e2e.js
const detox = require('detox');
const config = require('../package.json').detox;

Expand All @@ -44,15 +43,15 @@ beforeEach(async () => {
Add this part to your `package.json`:
```json
"jest": {
"setupTestFrameworkScriptFile": "<rootDir>/jest/setup.js"
"setupTestFrameworkScriptFile": "./e2e/init.js"
},
"scripts": {
"test:e2e": "jest __e2e__ --setupTestFrameworkScriptFile=./jest/setup/e2e-tests.js --runInBand",
"test:e2e": "detox test",
"test:e2e:build": "detox build"
}
```

We need the `--runInBand` as detox doesn't support parallelism yet.
Copy link
Member

Choose a reason for hiding this comment

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

Will running without the flag run tests serially?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the flag here so if you use detox test -r jest it is always there

Copy link
Contributor

Choose a reason for hiding this comment

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

What is stopping detox from parallelism?

Copy link
Member

Choose a reason for hiding this comment

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

a few things, mainly controlling multiple simulators. It's in our roadmap, shouldn't be too hard.

In the `detox` part of your `package.json`, add `"runner": "jest"` to tell detox that you want to use jest runner instead of mocha.

### Writing Tests

Expand Down