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

test: Add integration(e2e) test #424

Merged
merged 24 commits into from
Oct 28, 2019
Merged

test: Add integration(e2e) test #424

merged 24 commits into from
Oct 28, 2019

Conversation

hanwencheng
Copy link
Contributor

@hanwencheng hanwencheng commented Oct 17, 2019

This is the configuration part of #326 which enable us to run CI locally.

For test it locally you need install detox-cli as global npm dependency. it has conflict with local detox dependency so that we could not add it locally and start with npx

npm install -g detox-cli

After testing with Cavy and Detox, I find Detox is a better choice for us, since we want to develop the test and the origin code base at the same time. And the only complex part of Detox is the configuration part, to use it we do not to use High Order Component, but just with an testID on the component. It is more powerful and It is in active development by Wix team.

There are some comparison articles like Detox vs Appium or Detox vs Cavy

c37c6e4 : fixes a recursive path problem caused by react-native-camera when build test on iOS, discussion here

Test Demo

iOS Android
out out_android

Further steps

After a lot of attempts trying to add the integration test on Travis CI, I keep get the failing test because the build process failed without variant configurations like Rust environment or Android NKD. This will need helps from other people who is experienced with setting Travis CI test environment, and I would like to suggest now we do the CI test locally, and work on Travis CI integration in another PR, to prevent blocking current development work.

@hanwencheng hanwencheng self-assigned this Oct 17, 2019
@@ -107,7 +108,7 @@ android {

defaultConfig {
applicationId "io.parity.signer"
minSdkVersion 16
minSdkVersion 18
Copy link
Contributor Author

Choose a reason for hiding this comment

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

minimal version for Detox

afterEvaluate {project ->
if (project.hasProperty("android")) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fix the dependency version problem, e.g. in react-native-secure-store the compiledSdkVersion is 27, which will leads a compiling error.

@hanwencheng hanwencheng changed the title WIP: test: Add integration(e2e) test test: Add integration(e2e) test Oct 19, 2019
Copy link
Contributor

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

Thanks a lot for that. Any chance this will work with a real device? I get the following:

yarn test-e2e:android
yarn run v1.19.1
$ detox test -c android.emu.debug
detox[8550] INFO:  [test.js] configuration="android.emu.debug" artifactsLocation="artifacts/android.emu.debug.2019-10-21 09-57-31Z" recordLogs="none" takeScreenshots="manual" recordVideos="none" recordPerformance="none" reportSpecs=true readOnlyEmu=false node_modules/.bin/jest --config=e2e/config.json --maxWorkers=1 '--testNamePattern=^((?!:ios:).)*$' "e2e"
detox[8558] INFO:  [DetoxServer.js] server listening on localhost:41537...
detox[8558] ERROR: [DetoxExportWrapper.js/DETOX_INIT_ERROR] 
 Error: Can not boot Android Emulator with the name: 'Nexus_5_API_28',
      make sure you choose one of the available emulators: 
    at EmulatorDriver._validateAvd (/home/thib/github/paritytech/parity-signer/node_modules/detox/src/devices/drivers/EmulatorDriver.js:98:13)
Load test: should have account list screen
Load test: should have account list screen [FAIL]

 FAIL  e2e/firstTest.spec.js
  Load test
    ✕ should have account list screen (2ms)

  ● Load test › should have account list screen

    Can not boot Android Emulator with the name: 'Nexus_5_API_28',
          make sure you choose one of the available emulators:

      at EmulatorDriver._validateAvd (../node_modules/detox/src/devices/drivers/EmulatorDriver.js:98:13)

  ● Load test › should have account list screen

    ReferenceError: device is not defined

      3 | describe('Load test', () => {
      4 |   beforeEach(async () => {
    > 5 |     await device.reloadReactNative();
        |           ^
      6 |   });
      7 | 
      8 |   it('should have account list screen', async () => {

      at device (firstTest.spec.js:5:11)
      at tryCatch (../node_modules/regenerator-runtime/runtime.js:45:40)
      at Generator.invoke [as _invoke] (../node_modules/regenerator-runtime/runtime.js:271:22)
      at Generator.prototype.(anonymous function) [as next] (../node_modules/regenerator-runtime/runtime.js:97:21)
      at tryCatch (../node_modules/regenerator-runtime/runtime.js:45:40)
      at invoke (../node_modules/regenerator-runtime/runtime.js:135:20)
      at ../node_modules/regenerator-runtime/runtime.js:170:11
      at callInvokeWithMethodAndArg (../node_modules/regenerator-runtime/runtime.js:169:16)
      at AsyncIterator.enqueue (../node_modules/regenerator-runtime/runtime.js:192:13)
      at AsyncIterator.prototype.(anonymous function) [as next] (../node_modules/regenerator-runtime/runtime.js:97:21)
      at Object.<anonymous>.exports.async (../node_modules/regenerator-runtime/runtime.js:216:14)
      at Object._callee (firstTest.spec.js:4:14)

  ● Load test › should have account list screen

    ReferenceError: element is not defined

       6 |   });
       7 | 
    >  8 |   it('should have account list screen', async () => {
         |                                         ^
       9 |     await expect(element(by.id(testIDs.TacScreen.tacView))).toBeVisible();
      10 |     await element(by.id(testIDs.TacScreen.agreePrivacyButton)).tap();
      11 |     await element(by.id(testIDs.TacScreen.agreeTacButton)).tap();

      at _callee2$ (firstTest.spec.js:8:41)
      at tryCatch (../node_modules/regenerator-runtime/runtime.js:45:40)
      at Generator.invoke [as _invoke] (../node_modules/regenerator-runtime/runtime.js:271:22)
      at Generator.prototype.(anonymous function) [as next] (../node_modules/regenerator-runtime/runtime.js:97:21)
      at tryCatch (../node_modules/regenerator-runtime/runtime.js:45:40)
      at invoke (../node_modules/regenerator-runtime/runtime.js:135:20)
      at ../node_modules/regenerator-runtime/runtime.js:170:11
      at callInvokeWithMethodAndArg (../node_modules/regenerator-runtime/runtime.js:169:16)
      at AsyncIterator.enqueue (../node_modules/regenerator-runtime/runtime.js:192:13)
      at AsyncIterator.prototype.(anonymous function) [as next] (../node_modules/regenerator-runtime/runtime.js:97:21)
      at Object.<anonymous>.exports.async (../node_modules/regenerator-runtime/runtime.js:216:14)
      at Object._callee2 (firstTest.spec.js:8:41)

detox[8550] ERROR: [cli.js] Error: Command failed: node_modules/.bin/jest --config=e2e/config.json --maxWorkers=1 '--testNamePattern=^((?!:ios:).)*$' "e2e"

I had the issue both with yarn start running in another shell and without

README.md Outdated Show resolved Hide resolved
.travis.yml Outdated Show resolved Hide resolved
@hanwencheng
Copy link
Contributor Author

I have tested it on Emulator, it is also possible to use real device, but need some configuration, basically we have to override device settings with device name, I will make an update on it soon.

The device should be listed in the output of adb devices command under provided name. Use this type to connect to Genymotion emulator.

@hanwencheng
Copy link
Contributor Author

Tried on real device, but do not has the luck to run it success on device. It seems to be a problem of Detox with support of RN 0.60.x, will track the problem on this issue wix/Detox#1689

Copy link
Contributor

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

I can't review this PR further since I do not have any emulator installed.
@yjkimjunior or @sjeohp can you have a look?

Since this doesn't support physical devices, we should mention it and remove anything that tells what to do with a real device since it doesn't work.

@pmespresso
Copy link
Contributor

pmespresso commented Oct 23, 2019

@hanwencheng not sure if this is an Xcode version mismatch issue, but my binary path is at: ./ios/build/Build/Products/Debug-iphonesimulator/NativeSigner.app, as opposed to ios/build/NativeSigner/Build/Products/Debug-iphonesimulator/NativeSigner.app which led to the error I messaged you about earlier

The test itself works well otherwise

Copy link
Contributor

@pmespresso pmespresso left a comment

Choose a reason for hiding this comment

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

as discussed offline, looks like the xcode build and detox build artifacts are mismatched, so will need an extra script to check that as a workaround, potentially to be fixed in the detox repo directly.

everything else went smoothly for me. works well on android

Copy link
Contributor

@pmespresso pmespresso left a comment

Choose a reason for hiding this comment

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

latest commit fixes my grumble

@Tbaut Tbaut self-requested a review October 23, 2019 15:02
Copy link
Contributor

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

My review is barely useful but my grumbles are addressed :)

@sjeohp-zz
Copy link
Contributor

I can't review this PR further since I do not have any emulator installed.
@yjkimjunior or @sjeohp can you have a look?

Detox build gives me a linker error for ios: library not found for -lRNSecureStorage. Just that lib for some reason, the others are fine.

@pmespresso
Copy link
Contributor

@sjeohp it looks like you need to link react-native-secure-storage again, maybe the link was lost while checking out the branch. react-native link react-native-secure-storage should do it.

@hanwencheng hanwencheng merged commit 0ba9966 into master Oct 28, 2019
@hanwencheng hanwencheng deleted the hanwen-integration-test branch October 28, 2019 12:47
@@ -146,6 +149,8 @@ android {
release {
minifyEnabled enableProguardInReleaseBuilds
proguardFiles getDefaultProguardFile("proguard-android.txt"), "proguard-rules.pro"
// Detox-specific additions to pro-guard
// proguardFile "${rootProject.projectDir}/../node_modules/detox/android/detox/proguard-rules-app.pro"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we remove this stray 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.

These code is needed once ProGuard is Enabled, I will add one line comment above.

pmespresso pushed a commit that referenced this pull request Nov 9, 2019
* init detox e2e test

* Add first e2e test

* add android configurations

* fix detox rn-camera problem

* add CI integration

* update ci settings

* update xcode version

* use legacy build in CI

* prettier xcode output

* update with android

* revert back to origin CI test

* add E2E test in readme

* Update .travis.yml

* update detox guide

* renaming

* update readme

* Update README.md

Co-Authored-By: Thibaut Sardan <33178835+Tbaut@users.noreply.github.com>

* Update .travis.yml

* Update README.md

* fix binary path problem

* remove before each reloading

* rephrase
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants