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 9 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
10 changes: 9 additions & 1 deletion detox/.eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -400,5 +400,13 @@
"promise/no-promise-in-callback": "error",
"promise/no-callback-in-promise": "off",
"promise/avoid-new": "off"
}
},
"overrides": [
{
"files": "scripts/**",
"rules": {
"no-console": "off"
}
}
]
}
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'});
6 changes: 4 additions & 2 deletions detox/local-cli/detox-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ function runMocha() {
const platform = program.platform ? `--grep ${getPlatformSpecificString(program.platform)} --invert` : '';

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

console.log(command);
cp.execSync(command, {stdio: 'inherit'});
Expand All @@ -83,7 +84,8 @@ function runMocha() {
function runJest() {
const configFile = runnerConfig ? `--config=${runnerConfig}` : '';
const platform = program.platform ? `--testNamePattern='^((?!${getPlatformSpecificString(program.platform)}).)*$'` : '';
const command = `node_modules/.bin/jest ${testFolder} ${configFile} --runInBand ${platform}`;
const binPath = path.join('node_modules', '.bin', 'jest');
const command = `${binPath} ${testFolder} ${configFile} --runInBand ${platform}`;
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,13 +21,13 @@
"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",
"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
30 changes: 30 additions & 0 deletions detox/scripts/build.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#!/usr/bin/env node

// Javascript for windows support.

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);
});

childProcess.execFileSync(
"android/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.

8 changes: 8 additions & 0 deletions detox/scripts/postinstall.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#!/usr/bin/env node

// Javascript for windows support.
Copy link
Member

Choose a reason for hiding this comment

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

Unnecessary comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

More clearly Using JS instead of shell script to avoid breaking on windows, but I'm happy to remove it!

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.

13 changes: 10 additions & 3 deletions detox/src/devices/EmulatorDriver.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,17 @@ 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);
}

adbDevices = await this.adb.devices();
Copy link
Member

Choose a reason for hiding this comment

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

Why is this being run twice ?
Does the original implementation cause issues with Windows ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, should be inside the if here - it's refreshing adbDevices after the requested name is booted. See the comments above as to why this is happening, but roughly: on windows trying to start the emulator the way it currently is being done when it's already running causes issues for some reason.

filteredDevices = _.filter(adbDevices, {type: 'emulator', name: name});

let adbName;
switch (filteredDevices.length) {
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