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

Usage with Expo instructions #630

Merged
merged 8 commits into from
Jun 21, 2018

Conversation

peterpme
Copy link
Contributor

No description provided.

@adam-stasiak
Copy link

Hey, Could you add please an instruction for Android setup? I have already raised an issue #578 and my team cannot proceed with android tests.

Copy link
Member

@rotemmiz rotemmiz left a comment

Choose a reason for hiding this comment

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

Thanks for that!

I am wondering if you ever tried to do the same for Android, I'm not sure it's even possible (since Android build needs to generate a test APK alongside the app APK, and both should be singed with the same keystore)

```

- Download the Expo app from [Expo.io/tools](https://expo.io/tools). As of 03/19/18
- [iOS IPA 2.3.0](https://dpq5q02fu5f55.cloudfront.net/Exponent-2.3.0.tar.gz)
Copy link
Member

Choose a reason for hiding this comment

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

Try being less specific here, we don't want to update this regularly. Just put a link to the download page (https://expo.io/tools)

- [iOS IPA 2.3.0](https://dpq5q02fu5f55.cloudfront.net/Exponent-2.3.0.tar.gz)
- [Android APK 2.3.2](https://d1ahtucjixef4r.cloudfront.net/Exponent-2.3.2.apk)

- Unzip the iOS IPA and **rename the folder** to `Exponent.app` (it'll now be a file and not a folder)
Copy link
Member

Choose a reason for hiding this comment

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

It will now have a file icon (though is still a folder)

@peterpme
Copy link
Contributor Author

Thanks @rotemmiz!

@rotemmiz
Copy link
Member

Hey @peterpme ,

Can you please relate to both platforms ? even if Android is not currently working, or you're not sure how to, just mention it in docs, se we'd take it from there.

@peterpme
Copy link
Contributor Author

I added a TBD section for Android

@rotemmiz
Copy link
Member

Please rephrase the Android part, it seems more like a personal note than documentation. 😯

- We haven't personally tried getting this to work on Android. If you have, feel free to open up a PR!

### Example App
The [example app](https://github.com/expo/with-detox-tests) is outdated but the code is exactly the same. I have a PR open that'll make it runnable again.
Copy link
Member

Choose a reason for hiding this comment

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

This is the line I was referring to

@peterpme
Copy link
Contributor Author

👋 @rotemmiz anything else?

@rotemmiz
Copy link
Member

Hey @peterpme , looks like your PR was accepted in expo, right?
Please change the note.

@DanielMSchmidt
Copy link
Contributor

@peterpme Any updates here? 😇

@peterpme
Copy link
Contributor Author

peterpme commented May 2, 2018

@DanielMSchmidt how's that? Thanks!

const { reloadApp } = require('detox-expo-helpers');
// ...
beforeEach(async () => {
await reloadApp();
Copy link

Choose a reason for hiding this comment

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

Out of curiosity, is it possible at all to not have to reload the app in between every test? The setup/teardown process adds a fairly significant amount of time to test runs, but when I remove this beforeEach hook from my tests the tests get screwy, so I was wondering if this is just the way it is for now.

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 think this is just the way it has to be for now.

@ghost
Copy link

ghost commented Jun 14, 2018

Hi @peterpme, thanks a lot for this from a CRNA user!
I am getting close, but it still seems that the tests just time out. Is maybe some step missing?

@peterpme
Copy link
Contributor Author

Hey @CodingBauer I'm not entirely sure what your issues can be. It's been a couple months since I've opened the PR and a bunch may have changed 😅

@rotemmiz
Copy link
Member

Hey @peterpme,
Sorry for getting back to this after such a long time, I wasn't sure the addition was precise enough, therefore didn't merge it back then, but on the other hand didn't communicate my doubts, and I am sorry for that.
So, to fix what's I have broken, let's discuss:

  1. Since things might have change (or already have been changed), and maybe even because of that, try minimizing any expo related explanation and setup (e.g, bin download), instead use links, like you already have.
  2. Try keeping the text professional. Don't use "The most important piece of this" or "don't forget". I know we do it some places ourselves (and we shouldn't), but we want to make high quality docs.
  3. No need to mention specific twitter accounts or users.

I would gladly accept this docs addition, if you can please help aligning the text to these guidelines.

BTW, I think it would be a good idea to actually post these, and maybe a few more, guidelines to contribution guide (we need specific guidelines for each part I guess: docs, cross platform features, sync issues, tests, etc).

@peterpme
Copy link
Contributor Author

You got it @rotemmiz!!

I'll make these adjustments over the next couple of days and we can try and get this in :)

Thanks for taking the time to write that out ❤️

@peterpme
Copy link
Contributor Author

The changes were actually quick enough that I just took care of them.

I don't know how to explain this without keeping the instructions to download the bin files though.

Those steps were really helpful for me when I figured them out and if I could save a developer 15-30
minutes from scratching their heads and wondering how to do it, I think its worth it in the documentation.

Either way - entirely up to you! Thanks again!

@rotemmiz rotemmiz merged commit 84c9aef into wix:master Jun 21, 2018
@Srikanth-Pusapati
Copy link

Srikanth-Pusapati commented Jul 2, 2018

Can you update this file possibly with how to use Detox using Expo on an Android device?
Thanks.

@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.

6 participants