[ISSUE-78] Test orchestrator support

mariuszmarzec commented 2 years ago

:pushpin: References

Issues:
78
262

:tophat: What is the goal?

Providing support for AndroidX Orchestrator

How is it being implemented?

Compose testing is done by one of the contributor, so we have to only ensure preserving of metadata files. According to default screenshot done by facebook it is needed to move screenshots and metadata files to another temporory dir, because in every starting test screenshot tool from facebook clears screenshots-default dir.

Upgrading facebook to 0.14.0 version (Now metadata from facebook are save as json file instead of xml)
For default screenshots testing, copying screenshots and metadata files for every finished test into screenshots-default-orchestrated dir on device
For compose screenshots testing, copying metadata (screenshots for compose are safe during test with orchestration, no needs to moving) for every finished test into screenshots-default-compose-orchestrated dir on device

How can it be tested?

To test it, it would be worth to run record and verification on projects with/without orchestrator.

[x] Use case 1: Testing in all shot-consumer-* with orchestrator and without it

mariuszmarzec commented 2 years ago

@pedrovgs I have created a PR as a draft. Unit test are broken and code style. I have to fix it, but not having time in this week. Waiting for some feedback if something could be done better.

mariuszmarzec commented 2 years ago

@pedrovgs Implementation is finished. Last thing which should be done is testing on all shot-consumers projects with added/not added orchestrator.

stetro commented 2 years ago

Hey, thanks for introducing this, I will test this PR tomorrow in our project which also rely on orchestrator. 👍 I'll keep you updated.

stetro commented 2 years ago

I just tried this PR with no success, maybe I did it wrong:

Cloned your repo, checkout your branch
RELEASE_SIGNING_ENABLED=false and incremented the version for local testing purpose
./gradlew publishToMavenLocal and used the "incremented" version in my project
./gradlew clean executeScreenshotTests -Precord but still only one picture was recorded

mariuszmarzec commented 2 years ago

@stetro Looks like getting library from remote repository before maven local. It seems to be a matter of priority of repositories. I think you can moven maven local repository at first place (on the list in build.gradle) Or change version before release here https://github.com/pedrovgs/Shot/blob/121affae7874334754c68c9fd4e09bc9b4e3c09c/gradle.properties and update in target project to be 100% sure that you are using version with my changes.

mariuszmarzec commented 2 years ago

@pedrovgs Build failed with on one of shot-consumer project. Looks like something is wrong with this example or shot in 5.12.2, but it doesn't come from current version with my changes. We will try to test it on local machines as previous.

stetro commented 2 years ago

@stetro Looks like getting library from remote repository before maven local. It seems to be a matter of priority of repositories. I think you can moven maven local repository at first place (on the list in build.gradle) Or change version before release here https://github.com/pedrovgs/Shot/blob/121affae7874334754c68c9fd4e09bc9b4e3c09c/gradle.properties and update in target project to be 100% sure that you are using version with my changes.

Correct that's why I increased the version in step 2. To be totally sure :)

mariuszmarzec commented 2 years ago

@stetro Ahh, right. Both version name and version code? If yes, looks like i will have to check it again.

stetro commented 2 years ago

@stetro Ahh, right. Both version name and version code? If yes, looks like i will have to check it again.

Correct, I incremented both

mariuszmarzec commented 2 years ago

@stetro Ok, thanks for effort, will check when i find some time.

mariuszmarzec commented 2 years ago

@stetro I fixed all issues i found. Could you check it? @pedrovgs I have tested on all shot-consumer-* project and it worked.

stetro commented 2 years ago

@stetro I fixed all issues i found. Could you check it? @pedrovgs I have tested on all shot-consumer-* project and it worked.

With this I unfortunately run into We couldn't find any screenshot. :see_no_evil:

mariuszmarzec commented 2 years ago

@stetro is your project public maybe? Are you using composer? I found that composer with orchestrator doesn't work. Could you check it on emulator API 28? I remember that there is another issue on 29+

stetro commented 2 years ago

@stetro is your project public maybe? Are you using composer? I found that composer with orchestrator doesn't work. Could you check it on emulator API 28? I remember that there is another issue on 29+

Its not using compose. Running on API 30 (only) and also a private repo :see_no_evil: Testing with another emulator is not possible as well - If my usecase is too complex to verify and debug this, never mind, I just thought someone testing it would be helpful :)

mariuszmarzec commented 2 years ago

@stetro I meant composer (https://github.com/composer/composer), it is different tool. But information about not using compose is important as well. I started to think something could be wrong with facebook 0.14.0, but it needs more investigation. 🤷‍♂️

stetro commented 2 years ago

@stetro I meant composer (https://github.com/composer/composer), it is different tool. But information about not using compose is important as well. I started to think something could be wrong with facebook 0.14.0, but it needs more investigation. man_shrugging

I hope you mean https://github.com/gojuno/composer :speak_no_evil: :D Bot no, thats also not used.

mariuszmarzec commented 2 years ago

@pedrovgs Have checked composer case and provided instruction to readme how to deal with composer + orchestrator. I have check test-consumer-* on API 28 with disabled and enabled orchestrator, it works. I check it in my private project also and working fine.

@stetro Yeah, copied wrong link. According your issues on API 30. Is there any suspicious logs in gradlew or in logcat? I found that could be a issue with hamcrest, because shot-android is using espresso 3.30, i had kaspresso which used 3.4.0 under the hood and it makes tests failed. Whole executeScreenshotTests tasks finished successfully but tests didn't make screenshots due to failing.

mariuszmarzec commented 2 years ago

@stetro Done testing on API 30 moment ago and works for me. Check your dependencies. I used same as used in shot-android module:

    val runner_version = "1.3.0"
    val orchestrator_version = "1.4.1"
    val android_test_services_version = "1.4.1"
    val espresso_core = "3.3.0"

    androidTestImplementation("androidx.test:core:${Dependency.runner_version}")
    androidTestImplementation("androidx.test:runner:${Dependency.runner_version}")
    androidTestImplementation("androidx.test:rules:${Dependency.runner_version}")
    androidTestUtil("androidx.test:orchestrator:${Dependency.orchestrator_version}")
    androidTestUtil("androidx.test.services:test-services:${Dependency.android_test_services_version}")
    androidTestImplementation("androidx.test.espresso:espresso-core:${Dependency.espresso_core}")
    androidTestImplementation("androidx.test.espresso:espresso-contrib:${Dependency.espresso_core}")

Some of them could be updated in shot-android to newest versions but i don't want to make this PR bigger. Updates it is good thing for next enhancement.

pedrovgs commented 2 years ago

Hey guys sorry for the late response :( I've got covid and couldn't check this for a while. Sorry

pedrovgs commented 2 years ago

@mariuszmarzec I'm reading your comments and contributions and the only thing I can say is WOW and CONGRATS. I need more time to review the details but looks like everything is in place. Could you add another step to the CI to re-run the tests with composer enabled so we can ensure we don't break it in the future? Maybe we can use existing shot-consumer projects and change the build.gradle config based on an environment variable, what do you think?

mariuszmarzec commented 2 years ago

@pedrovgs No problem, hope you are feel better. thanks for appreciation :)

Composer is not a case. I thought i broke something, but i was wrong, composer doesn't use orchestrator by default and it was needed only to provide configuration to composer and enable it, i have added guide how to do it in README.

But i think it will be worth to add to every shot-consumer orchestrator without enable it and i will add to CI configuration steps to rerun all builds with enabling orchestration by command param

------EDIT There is no convenient api to disable/enable orchestrator. Maybe best option will be enabling for orchestrator for one of shot-conusmer project?

------EDIT I have worse day today, you mentioned about setting variable, seems to be best option

mariuszmarzec commented 2 years ago

@pedrovgs Added CI check with enabled orchestrator. Work is completely done. 🎉 If any code issues to fix, of course will fix it, waiting for comments or for merge and new version release 😄

mariuszmarzec commented 2 years ago

@pedrovgs Hey, happy to see new release 😄 I don't have any twitter account unfortunately, no problem if there will be no contact, i was mentioned on release notes, so it is good too 😄

pedrovgs / Shot