Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

review of practices #30

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

Open
sergio-sastre wants to merge 13 commits into main
base: main
Choose a base branch
Loading
from practices_review
Open

review of practices #30

sergio-sastre wants to merge 13 commits into main from practices_review

Conversation

Copy link
Member

@sergio-sastre sergio-sastre commented Dec 7, 2021 •
edited
Loading

For now this includes a review of:

  • State clearing
  • Test Runners
  • Emulators vs devices
  • Emulator setup
  • PageObject pattern
    ...

more to come soon :)

@@ -58,10 +63,10 @@ them [here](https://github.com/AdevintaSpain/Barista/tree/master/library/src/mai

!!! warning

This solution won't in 100% of cases:
This solution won't work in 100% of cases:

1. You may have runtime cache, which can also affect your tests
Copy link
Member Author

@sergio-sastre sergio-sastre Dec 7, 2021

Choose a reason for hiding this comment

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

I did not get this point with the runtime cache. What is stored in there and how it affects? Maybe an example would help

Copy link
Member

Choose a reason for hiding this comment

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

Imagine if we store our data in databases and in runtime cache, like hashmap
Clearing of internal storage won't clear it. We have to clear it somehow, the only way -- use some production code, which is a bottleneck

sergio-sastre reacted with thumbs up emoji
uses [special services.](https://github.com/android/android-test/tree/master/services)
It's just a shell client and should be installed to the device.
Orchestrator also needs to execute adb commands. For that it uses [special services.](https://github.com/android/android-test/tree/master/services) under the hood.
It's just a shell client and should be installed on the device.
Copy link
Member Author

@sergio-sastre sergio-sastre Dec 7, 2021

Choose a reason for hiding this comment

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

After reading this, I was doubting whether I have to install it on my own, or the Orchestrator does it for me (this one right?). This should be clarified

@@ -118,15 +119,15 @@ It's just a shell client and should be installed to the device.
Despite the fact that it does the job, this solution looks overcomplicated:

1. We need to install +2 different apk to each emulator
2. We delegate this job to the device instead of host machine.
2. We delegate this job to the device instead of the host machine.
<br/>Devices are less reliable than host pc
Copy link
Member Author

@sergio-sastre sergio-sastre Dec 7, 2021

Choose a reason for hiding this comment

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

Less reliably in the sense of "they do not lose connection to adb server, issues with battery, internet, system dialogs," or sth like that?

Sergio Sastre Flórez added 2 commits December 7, 2021 22:20
<br/>
➖ Slow execution _(can take 10+ seconds and depends on apk size)_ <br/>
Copy link
Member Author

@sergio-sastre sergio-sastre Dec 7, 2021

Choose a reason for hiding this comment

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

10+ was for ALL the tests together due to adb pm clear right? This I did not understand very well

➕ Fast execution in the same process<br/>
<br/>
➖ Don't give you any guarantee that your app will be cleared properly<br/>
➖ Application or Test process killing will break tests execution <br/>
➖ Can be a bottleneck<br/>
➖ Can be a bottleneck<br/>
Copy link
Member Author

@sergio-sastre sergio-sastre Dec 7, 2021

Choose a reason for hiding this comment

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

this needs some short hint of why



* Impossibility to scale
<br>
_As soon as you started your tests, it's impossible to connect more devices to tests run on fly_
_As soon as you started your tests, it's impossible to let more devices join the test execution on the fly_
Copy link
Member Author

@sergio-sastre sergio-sastre Dec 8, 2021

Choose a reason for hiding this comment

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

I think explaining why it can help to add test on the fly and what is the case would help.

I was thinking about running test on a device cloud? There could be more available after a while? @NoNews what is the case exactly?

➕ adb client `ddmlib` replacement: [Adam](https://github.com/Malinskiy/adam) <br>
➕ Cross-platform (iOS support) <br>
➕ Fragmented test execution (similar to AOSP's sharding): split large testing suites into multiple CI builds <br>
➕ Parallel execution of parameterised tests <br>
➕ Interactions with adb/emulator from within a test (e.g. fake fingerprint or GPS) <br>
➕ Interactions with adb/emulator from within a test (e.g. fake fingerprint or GPS) <br>
Copy link
Member Author

@sergio-sastre sergio-sastre Dec 8, 2021

Choose a reason for hiding this comment

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

Looks like Kaspresso does already half of these things :P

➕ Pull files from the device after test run,
e.g. [allure-kotlin](https://github.com/allure-framework/allure-kotlin) <br>
➕ Basic [Allure](https://github.com/allure-framework) support out of the box <br>
➕ Basic [Allure](https://github.com/allure-framework) support out of the box <br>
Copy link
Member Author

@sergio-sastre sergio-sastre Dec 8, 2021

Choose a reason for hiding this comment

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

It is not clear what Allure does for someone who has not used it:
video (what is mentioned above), reports...
I'd rather mention the most relevant, that have not been mentioned above, or exclude video form above and put it here (can Marathon record videos without Allure?) @NoNews


After that, you can run your emulator by `avd manager`, which is a part of android sdk manager. After your device
creation, you need change default generated ini file to custom one. You may see an example below:
After that, you can run your emulator via `avd manager`, which is part of the android sdk manager. After creating the emulator, you need to switch the default generated `ini` file to the custom one we defined previously.
Copy link
Member Author

@sergio-sastre sergio-sastre Dec 10, 2021

Choose a reason for hiding this comment

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

I rephrased this. I believe this is what is intended after reading the code in the script

Talking about [Avito emulator](https://github.com/google/android-emulator-container-scripts), it also patches your
emulator with adb commands to prevent tests flakiness and to speed them up
Talking about the [Avito emulator](https://github.com/google/android-emulator-container-scripts), it also patches your
emulator with adb commands to prevent tests flakiness and to speed them up.
Copy link
Member Author

@sergio-sastre sergio-sastre Dec 10, 2021

Choose a reason for hiding this comment

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

would be good to quickly mention how it does it.
From the description here, I take for granted that Avito emulator is the best out of the 3 here? If yes, should we stress the point in here?

What are the differences among them?

As a learner, I want to use the best for my needs

After that, you can run your emulator by `avd manager`, which is a part of android sdk manager. After your device
creation, you need change default generated ini file to custom one. You may see an example below:
After that, you can run your emulator via `avd manager`, which is part of the android sdk manager. After creating the emulator, you need to switch the default generated `ini` file to the custom one we defined previously.
You can achieve that with a script like this one:

```bash
function define_android_sdk_environment_if_needed() {
Copy link
Member Author

@sergio-sastre sergio-sastre Dec 10, 2021

Choose a reason for hiding this comment

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

I strongly recommend to have a "ready" bash file for this that we upload in this article (maybe extra subsection here to make it more prominent) and indicate how to execute -> ./bashFile.sh -args
The command can actually differ on windows though

@@ -109,22 +109,28 @@ define_path_environment_if_needed
create_and_patch_emulator
```

Pay your attention that you also need to wait until your emulator is fully booted.
Keep in mind that the emulator must fully boot before running any test. Otherwise the tests will fail because there is still no device ready
Copy link
Member Author

@sergio-sastre sergio-sastre Dec 10, 2021

Choose a reason for hiding this comment

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

Actually, for snapshot testing I was using swarmer from Juno, which is deprecated but works pretty fine for starting several emulators and wait for all of them to be ready before running the tests.
Another new thing is Gradle Virtual Managed Devices...

Maybe worth mentioning later, after releasing the MVP?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NoNews NoNews Awaiting requested review from NoNews

@v1sar v1sar Awaiting requested review from v1sar

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /