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

Fix: Detox is partially configured and is not working #38

Open
wants to merge 5 commits into
base: current
Choose a base branch
from

Conversation

andreaforni
Copy link
Contributor

@andreaforni andreaforni commented Aug 12, 2020

This PR closes #37

Please do a yarn install and then follow the instructions in the README.

…orrect the iOS configuration. Remove Hello World test and add "user age consent".

Android configuration is still missing.
…ing (the yellow box covers the button to click)
@andreaforni andreaforni self-assigned this Aug 12, 2020
Comment on lines +74 to +76
YellowBox.ignoreWarnings([
'Warning: componentWillReceiveProps has been renamed'
]);
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 had to disable this warning because the yellow box covers the Continue button in the "Your Data" screen of the Onboarding flow, blocking some tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not move these to the UNSAFE variants for the time being?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @dharding for the comment. I cannot, because that method is used in an imported library, not in the app code. The warning says:

Warning: componentWillReceiveProps has been renamed, and is not recommended for use. See https://fb.me/react-unsafe-component-lifecycles for details.

...

Please update the following components: Markdown

The src/components/atoms/markdown.tsx does not use that method but the imported M object from react-native-easy-markdown does.

And we are already using the latest version of the library (2.0.0). There isn't an updated version that fixes it.

Comment on lines +36 to +56
// The try/catch block tries to solve an edge case on Android:
// 1. The emulator is not running.
// 2. Detox runs the emulator and then the test.
// 3. Tapping the continue button calls, among other things, the Google SafetyNet Attestation API.
// 4. The attestation call fails with error code "7".
// 5. The app remains in Your Data screen and displays an error toast.
// Note: the attestation call does not fail if the emulator is already running before Detox runs the test.
// If the app doesn't move to the App Metrics screen, the test taps the continue button again.
// This time the attestation call should succeed.
const appMetricsTitle = by.text(en.appUsage.title);
try {
// Ensure the app metrics view is displayed.
await waitForElement(appMetricsTitle, 'toBeVisible', 90);
} catch (errNotFound) {
await waitFor(continueButtonEl)
.toBeVisible()
.whileElement(scrollViewId)
.scroll(100, 'down');
await continueButtonEl.tap();
await waitForElement(appMetricsTitle, 'toBeVisible');
}
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 try/catch block solves an issue happening with the Android emulator.

If the Android emulator is not running before Detox runs the test, some tests fail when the user clicks the Continue button in "Your Data" screen.
This is happening because the button triggers the register call (src/services/api/index.ts), which calls the verify function (src/services/api/index.ts). Here, the Google SafetyNet Attestation call fails with a cryptic [Error: 7: ]. At this point, the app remains in "Your Data" screen and displays a network error toast. The test fails because it expects to find the next screen title ("App Metrics"), but it doesn't.

To detect this error, I had to increase Detox expectation waiting time from 10 to 90 seconds because the attestation call takes about 60 seconds to fail on my machine.

I was not able to find a fix for the Error 7problem, I've checked the react-native-google-safetynet repo, and there is an open issue about this type of error (rajivshah3/react-native-google-safetynet#386). So let's wait to be clarified/fixed by the owner of the library.

I've put in place a workaround to avoid test failing in this particular case: I wait 90 seconds to see if the "App Metrics" title appears; if it doesn't, Detox throws an exception and the app is still in "Your Data" screen. I catch this exception, I click the Continue button and check the existence of the "App Metrics" title again. In my tests, the second Attestation call always works, so the test can continue.

Comment on lines +81 to +93
const appMetricsTitle = by.text(en.appUsage.title);
// For the explanation of this try/catch block, see the previous test.
try {
// Ensure the app metrics view is displayed.
await waitForElement(appMetricsTitle, 'toBeVisible', 90);
} catch (errNotFound) {
await waitFor(continueButtonEl)
.toBeVisible()
.whileElement(scrollViewId)
.scroll(100, 'down');
await continueButtonEl.tap();
await waitForElement(appMetricsTitle, 'toBeVisible');
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andreaforni andreaforni marked this pull request as ready for review August 12, 2020 15:28
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.

Fix: Detox is partially configured and is not working
2 participants