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

[Android] Enable usage of custom instrumentation test runners #675

Merged
merged 4 commits into from
Apr 24, 2018

Conversation

wiyarmir
Copy link
Contributor

  • Spawn adb shell and ask package manager for all runners
  • Regex the one for the package under test and use it

@wiyarmir wiyarmir requested a review from rotemmiz as a code owner April 18, 2018 23:14
@wiyarmir
Copy link
Contributor Author

closing and reopening to trigger another build

@wiyarmir wiyarmir closed this Apr 19, 2018
@wiyarmir wiyarmir reopened this Apr 19, 2018
@@ -72,6 +79,8 @@ class AndroidDriver extends DeviceDriverBase {
return this.instrumentationProcess.pid;
}

const testRunner = await this.adb.getInstrumentationRunner(deviceId, bundleId)
Copy link
Member

Choose a reason for hiding this comment

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

;

Copy link
Member

Choose a reason for hiding this comment

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

; missing

@@ -0,0 +1,37 @@
describe("AndroidDriver", () => {
Copy link
Member

Choose a reason for hiding this comment

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

Drivers are not unit tested on purpose. The mostly touch command line tools and are just wrappers. Unit tests don't give much value. They are being tested in the e2e suite.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function is doing some regex voodoo so that's why I thought about adding one at least, but I'm ok with removing it.

@@ -55,5 +55,10 @@ xdescribe('ADB', () => {
await adb.unlockScreen('deviceId');
expect(exec).toHaveBeenCalledTimes(1);
});

Copy link
Member

Choose a reason for hiding this comment

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

I guess we should consider dropping unit tests on ADB as well, but for now, don't delete this test

@wiyarmir
Copy link
Contributor Author

Took the freedom of fixing ADB tests a bit. Probably Parse 'adb device' output and getInstrumentationRunner are the only ones adding value at the moment, but I let them be. Have a look @rotemmiz

@wiyarmir wiyarmir force-pushed the android-fetch-runner-from-device branch from c8fcd6d to 86bfd02 Compare April 19, 2018 13:11
@wiyarmir wiyarmir closed this Apr 19, 2018
@wiyarmir wiyarmir reopened this Apr 19, 2018
@wiyarmir wiyarmir closed this Apr 24, 2018
@wiyarmir wiyarmir reopened this Apr 24, 2018
@rotemmiz rotemmiz merged commit a9bc3b8 into wix:master Apr 24, 2018
@wiyarmir wiyarmir deleted the android-fetch-runner-from-device branch April 24, 2018 09:53
wiyarmir added a commit to Skyscanner/detox that referenced this pull request Apr 24, 2018
* query pm and regex through results

* PR comments

* mock android SDK path

* More modularity and better test readability

(cherry picked from commit a9bc3b8)
@wix wix locked and limited conversation to collaborators Jul 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants