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

Running on windows support #628

Merged
merged 21 commits into from
May 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
989e45e
Update package scripts to use node for cross-plattform.
simonbuchan Mar 18, 2018
b950f2a
Fix detox test / run-server using paths that don't work on windows.
simonbuchan Mar 19, 2018
5982396
Replace CRLFs in exec.js, fixes running the emulator on windows.
simonbuchan Mar 19, 2018
4430484
Should fix build.js on mac.
simonbuchan Mar 19, 2018
9093c2d
Remove some paranoia.
simonbuchan Mar 19, 2018
a6169b2
Make eslint pass on JS scripts.
simonbuchan Mar 19, 2018
a50ebe7
Ignore exec hack in coverage to get CI passing.
simonbuchan Mar 19, 2018
3288a98
Walk back detox test change so it works with detox-cli.
simonbuchan Mar 19, 2018
2aace19
Don't try to start emulator if it's already running.
simonbuchan Mar 20, 2018
b0374e8
Merge remote-tracking branch 'upstream/master' into windows
simonbuchan Apr 18, 2018
d809041
Remove unneeded comments, only refresh ADB devices if needed.
simonbuchan Apr 18, 2018
80d20cb
Update detox unit tests to pass on windows.
simonbuchan Apr 24, 2018
3ff87ab
Add windows support to detox-cli.
simonbuchan Apr 24, 2018
0b6912c
Some basic documentation in roadmap.
simonbuchan Apr 24, 2018
8e4a325
Merge branch 'master' into windows
simonbuchan Apr 29, 2018
9e9fa8a
docs: 'detox' -> 'Detox'
simonbuchan May 7, 2018
dcc7bc5
More docs fixes.
simonbuchan May 7, 2018
86e7f7f
Merge branch 'master' into windows
simonbuchan May 7, 2018
7f03f0b
Oops, missed building Detox-ios-src.tbz
simonbuchan May 21, 2018
610c947
Merge remote-tracking branch 'upstream/master' into windows
simonbuchan May 22, 2018
140ad0e
Lint complained about hashbangs in detox scripts.
simonbuchan May 22, 2018
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
28 changes: 28 additions & 0 deletions detox-cli/cli.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
#!/usr/bin/env node
const cp = require('child_process');
const path = require('path');
const fs = require('fs');

const detoxPath = path.join(process.cwd(), 'node_modules/detox');
const detoxPackageJsonPath = path.join(detoxPath, 'package.json');

if (fs.existsSync(detoxPackageJsonPath)) {
// { shell: true } option seems to break quoting on windows? Otherwise this would be much simpler.
if (process.platform === 'win32') {
const result = cp.spawnSync(
'cmd',
['/c', path.join(process.cwd(), 'node_modules/.bin/detox.cmd')].concat(process.argv.slice(2)),
{ stdio: 'inherit' });
process.exit(result.status);
} else {
const result = cp.spawnSync(
path.join(process.cwd(), 'node_modules/.bin/detox'),
process.argv.slice(2),
{ stdio: 'inherit' });
process.exit(result.status);
}
} else {
console.log(detoxPackageJsonPath);
console.log("detox is not installed in this directory");
process.exit(1);
}
12 changes: 0 additions & 12 deletions detox-cli/cli.sh

This file was deleted.

2 changes: 1 addition & 1 deletion detox-cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"test": ":"
},
"bin": {
"detox": "./cli.sh"
"detox": "./cli.js"
},
"repository": {
"type": "git",
Expand Down
11 changes: 4 additions & 7 deletions detox/local-cli/detox-run-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,11 @@

const program = require('commander');
const cp = require('child_process');
const fs = require('fs');
const path = require('path');

program.parse(process.argv);

if (fs.existsSync(path.join(process.cwd(), 'node_modules/.bin/detox-server'))) {
Copy link
Member

Choose a reason for hiding this comment

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

npm and yarn install detox-server at different locations.
it might be either it node_modules or in node_modules/detox/node_modules

I am not familiar with require.resolve('detox-server/package.json'); trick. Will it cover both cases ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, it's the standard hack to find the location of an NPM package the same way node does, since every NPM package has a package.json. Remember:

  • require / require.resolve('foo/bar') looks up foo in node_modules, then adds bar to that path
  • without the .../bar it would use the main field, which is likely to be in a nested subdir
  • package.json always exists.

cp.execSync('node_modules/.bin/detox-server', {stdio: 'inherit'});
} else {
cp.execSync('node_modules/detox/node_modules/.bin/detox-server', {stdio: 'inherit'});
}

const serverPackagePath = require.resolve('detox-server/package.json');
const cli = require(serverPackagePath).bin['detox-server'];
const binPath = path.join(path.dirname(serverPackagePath), cli);
cp.execFileSync('node', [binPath], {stdio: 'inherit'});
7 changes: 5 additions & 2 deletions detox/local-cli/detox-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,16 +99,19 @@ function runMocha() {
const headless = program.headless ? `--headless` : '';

const debugSynchronization = program.debugSynchronization ? `--debug-synchronization ${program.debugSynchronization}` : '';
const command = `node_modules/.bin/mocha ${testFolder} ${configFile} ${configuration} ${loglevel} ${cleanup} ${reuse} ${debugSynchronization} ${platformString} ${artifactsLocation} ${headless}`;
const binPath = path.join('node_modules', '.bin', 'mocha');
const command = `${binPath} ${testFolder} ${configFile} ${configuration} ${loglevel} ${cleanup} ${reuse} ${debugSynchronization} ${platformString} ${artifactsLocation} ${headless}`;

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

function runJest() {
const configFile = runnerConfig ? `--config=${runnerConfig}` : '';
const platform = program.platform ? `--testNamePattern='^((?!${getPlatformSpecificString(program.platform)}).)*$'` : '';
const binPath = path.join('node_modules', '.bin', 'jest');
const platformString = platform ? `--testNamePattern='^((?!${getPlatformSpecificString(platform)}).)*$'` : '';
const command = `node_modules/.bin/jest ${testFolder} ${configFile} --runInBand ${platformString}`;
const command = `${binPath} ${testFolder} ${configFile} --runInBand ${platformString}`;
console.log(command);
cp.execSync(command, {
stdio: 'inherit',
Expand Down
6 changes: 3 additions & 3 deletions detox/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@
"author": "Tal Kol <talkol@gmail.com>",
"license": "MIT",
"scripts": {
"build": "scripts/build.sh",
"lint": "eslint src",
"build": "node scripts/build.js",
"lint": "eslint src scripts",
"unit": "jest --coverage --verbose",
"pretest": "npm run lint",
"test": "npm run unit",
"unit:watch": "jest --watch",
"prepublish": "npm run build",
"postinstall": "scripts/postinstall.sh"
"postinstall": "node scripts/postinstall.js"
},
"devDependencies": {
"eslint": "^4.11.0",
Expand Down
43 changes: 43 additions & 0 deletions detox/scripts/build.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
const childProcess = require('child_process');
const fs = require('fs-extra');

// Just make the usage a little prettier
function sh(cmdline, opts) {
const args = cmdline.split(' ');
const cmd = args.shift();
return childProcess.execFileSync(cmd, args, opts);
}

if (process.platform === 'darwin') {
console.log("\nPackaging Detox iOS sources");

fs.removeSync('Detox-ios-src.tbz');
// Prepare Earl Grey without building
sh("ios/EarlGrey/Scripts/setup-earlgrey.sh");
sh("find ./ios -name Build -type d -exec rm -rf {} ;");

sh("tar -cjf ../Detox-ios-src.tbz .", { cwd: "ios" });
}

if (process.argv[2] === "android" || process.argv[3] === "android") {
console.log("\nBuilding Detox aars");
const aars = [
"detox-minReactNative44-debug.aar",
"detox-minReactNative46-debug.aar",
"detox-minReactNative44-release.aar",
"detox-minReactNative46-release.aar"
];
aars.forEach(aar => {
fs.removeSync(aar);
});

sh("./gradlew assembleDebug assembleRelease", {
cwd: "android",
stdio: "inherit",
shell: true
});

aars.forEach(aar => {
fs.copySync(`android/detox/build/outputs/aar/${aar}`, aar);
});
}
29 changes: 0 additions & 29 deletions detox/scripts/build.sh

This file was deleted.

5 changes: 5 additions & 0 deletions detox/scripts/postinstall.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
if (process.platform === "darwin") {
require("child_process").execSync(`${__dirname}/build_framework.ios.sh`, {
stdio: "inherit"
});
}
5 changes: 0 additions & 5 deletions detox/scripts/postinstall.sh

This file was deleted.

2 changes: 1 addition & 1 deletion detox/src/configurations.mock.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ const pathsTests = {
},
"configurations": {
"absolutePath": {
"binaryPath": "/tmp/abcdef/123",
"binaryPath": process.platform === "win32" ? "C:\\Temp\\abcdef\\123" : "/tmp/abcdef/123",
"type": "ios.simulator",
"name": "iPhone 7 Plus, iOS 10.2"
},
Expand Down
7 changes: 4 additions & 3 deletions detox/src/devices/Device.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const _ = require('lodash');
const configurationsMock = require('../configurations.mock');

const path = require('path');

const validScheme = configurationsMock.validOneDeviceAndSession;
const invalidDeviceNoBinary = configurationsMock.invalidDeviceNoBinary;
const invalidDeviceNoDeviceName = configurationsMock.invalidDeviceNoDeviceName;
Expand Down Expand Up @@ -547,11 +548,11 @@ describe('Device', () => {

it(`should accept absolute path for binary`, async () => {
const actualPath = await launchAndTestBinaryPath('absolutePath');
expect(actualPath).toEqual('/tmp/abcdef/123');
expect(actualPath).toEqual(process.platform === 'win32' ? 'C:\\Temp\\abcdef\\123' : '/tmp/abcdef/123');
});

it(`should accept relative path for binary`, async () => {
const actualPath = await launchAndTestBinaryPath('relativePath');
expect(actualPath).toEqual(`${process.cwd()}/abcdef/123`);
expect(actualPath).toEqual(path.join(process.cwd(), 'abcdef/123'));
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be split to path.join(process.cwd(), 'abcdef', '123') ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Path will normalise separators, and windows in fine with either except for binary paths (where they are ambiguous with switches)

});
});
14 changes: 11 additions & 3 deletions detox/src/devices/EmulatorDriver.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,18 @@ class EmulatorDriver extends AndroidDriver {
}

await this._fixEmulatorConfigIniSkinName(name);
await this.emulator.boot(name);
Copy link
Member

Choose a reason for hiding this comment

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

What was the issue here ? boot should return a valid device, either it's shutdown or already booted. If it doesn't we may have issue with Emulator.js

Copy link
Member

Choose a reason for hiding this comment

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

I now see the comments here.

Not sure how to proceed here. This fix is in bad taste IMO.
So the real issue is creating and removing those emulator log files ?

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 could experiment a bit, I suspect redirecting to different files should fix it, but this was a less user visible change (eg. .gitignore)
Do you not find it logical to only start an emulator if it's not already running?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, gave it a try and it still hangs on startup with no output to the log file even if it's a new file each time.
Killing the first emulator then will hang it, killing it causes the second to start up and detox to continue successfully.
Seems like there's something lower level specific to the android emulator going on, possibly needs to have a separate AVD directory?


const adbDevices = await this.adb.devices();
const filteredDevices = _.filter(adbDevices, {type: 'emulator', name: name});
let adbDevices = await this.adb.devices();
let filteredDevices = _.filter(adbDevices, {type: 'emulator', name: name});

// If it's not already running, start it now.
if (!filteredDevices.length) {
await this.emulator.boot(name);

// Refresh devices after boot completes.
adbDevices = await this.adb.devices();
filteredDevices = _.filter(adbDevices, {type: 'emulator', name: name});
}

let adbName;
switch (filteredDevices.length) {
Expand Down
16 changes: 10 additions & 6 deletions detox/src/devices/android/APKPath.test.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
// Using path methods will normalize slashes to backslashes on win32, so tests must match.
const path = require('path');
const rootPath = process.platform === 'win32' ? 'C:\\Users\\SomeUser' : '~/somePath';

describe('APKPath', () => {
let APKPath;

Expand All @@ -6,20 +10,20 @@ describe('APKPath', () => {
});

it(`simple path`, async () => {
const inputApkPath = '~/somePath/build/outputs/apk/debug/app-debug.apk';
const expectedTestApkPath = '~/somePath/build/outputs/apk/androidTest/debug/app-debug-androidTest.apk';
const inputApkPath = path.join(rootPath, 'build/outputs/apk/debug/app-debug.apk');
const expectedTestApkPath = path.join(rootPath, 'build/outputs/apk/androidTest/debug/app-debug-androidTest.apk');
expect(APKPath.getTestApkPath(inputApkPath)).toEqual(expectedTestApkPath);
});

it(`path for a gradle build flavor`, async () => {
const inputApkPath = '~/somePath/build/outputs/apk/development/debug/app-development-debug.apk';
const expectedTestApkPath = '~/somePath/build/outputs/apk/androidTest/development/debug/app-development-debug-androidTest.apk';
const inputApkPath = path.join(rootPath, 'build/outputs/apk/development/debug/app-development-debug.apk');
const expectedTestApkPath = path.join(rootPath, 'build/outputs/apk/androidTest/development/debug/app-development-debug-androidTest.apk');
expect(APKPath.getTestApkPath(inputApkPath)).toEqual(expectedTestApkPath);
});

it(`path for a gradle build with multiple flavors`, async () => {
const inputApkPath = '~/android/app/build/outputs/apk/pocPlayStore/debug/app-poc-playStore-debug.apk';
const expectedTestApkPath = '~/android/app/build/outputs/apk/androidTest/pocPlayStore/debug/app-poc-playStore-debug-androidTest.apk';
const inputApkPath = path.join(rootPath, 'build/outputs/apk/pocPlayStore/debug/app-poc-playStore-debug.apk');
const expectedTestApkPath = path.join(rootPath, 'build/outputs/apk/androidTest/pocPlayStore/debug/app-poc-playStore-debug-androidTest.apk');
expect(APKPath.getTestApkPath(inputApkPath)).toEqual(expectedTestApkPath);
});

Expand Down
10 changes: 10 additions & 0 deletions detox/src/utils/exec.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,16 @@ async function execWithRetriesAndLogs(bin, options, statusLogs, retries = 10, in
// log.error(`${_operationCounter}: stderr:`, result.stderr);
//}

/* istanbul ignore next */
if (process.platform === 'win32') {
if (result.stdout) {
result.stdout = result.stdout.replace(/\r\n/g, '\n');
}
if (result.stderr) {
result.stderr = result.stderr.replace(/\r\n/g, '\n');
}
}

return result;
}

Expand Down
7 changes: 7 additions & 0 deletions docs/More.Roadmap.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ The current API supports addition of multiple platforms, Android is next. This i
### iOS physical device support
Currently Detox only supports running on iOS simulators, we plan on adding support for running on devices as well.

### Windows support
There is some work done for running Detox on Windows, but it's still fairly untested. Please open issues for anything you run into, but be aware of these limitations:

- Apple doesn't support iOS apps on Windows, so you're limited to the in-progress Android support.
- `binaryPath` can be left as a relative path with `/`, or use `\\` if you don't need cross-platform support.
- `build` should not use `./gradlew ...`, but simply `gradlew ...` - you may prefer scripting the build outside of Detox if you want to maintain cross-platform support - or simply have two configurations!

### Expectations on device logs
One of our most wanted features, being able to assert log outputs.

Expand Down