Skip to content

Add new smoke tests (Firestore) #319

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

Merged
merged 3 commits into from
Apr 2, 2019
Merged

Add new smoke tests (Firestore) #319

merged 3 commits into from
Apr 2, 2019

Conversation

allisonbm92
Copy link
Contributor

This pull request adds a new smoke test for Firestore and some infrastructure for the other smoke tests that be added soon. This pull request replaces #296.

The infrastructure is a little rough at this point, but it will be improved by the time we ask other developers to write their own tests.

This commit adds a new smoke test for Firestore and some infrastructure
for the other smoke tests that be added soon.
Copy link
Contributor

@ashwinraghav ashwinraghav left a comment

Choose a reason for hiding this comment

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

Thinking out loud.

Objectives / Design Goals

  1. Test the release infrastructure by testing released artifacts- proguard.txt, desugar.
  2. Test compatibility across SDKs and API boundaries.
  3. Have test code and testable code sit side by side.
  4. Run test code and testable code in test and app threads respectively.
  5. Package test code and testable code into test and app APK respectively.
  6. Test our products along with AGP's app extension (as opposed to library extension)
  7. Report meaningful SDK error instead of (say) "View could not be fond on screen".

Change #296

Pros

  • Test code was located side by side with code to be tested (referring to this as testable code henceforth).
  • Test code and testable code were packaged into corresponding, correct APKs.
  • Test code was run in the test thread. Testable code in the main thread.

Cons


Change #319

Pros

  • No new APIs need to be understood / maintained.

Cons

  • We have substituted the learning curve of the API with the unconventional pattern. Unconventional because:
    • Test libraries are in packaged the app APK.
    • App Code is being run on the test thread.
    • Nit: When we proguard these, we would have to preserve the @SmokeTest annotations.
    • Much like Add new smoke tests #296 , once we proguard, the usability of test results does reduce. This seems inevitable at this point.

With either of these solutions, there is a significant underlying design decision that the Android Test Frameworks team made, that we are violating by design.

For #296, all code is in the test APK, for the sake of colocation.
For #319, test code is in the app APK (for the sake of colocation) and firebase is run on the test thread to avoid the channels API.

I feel like colocating test code and testable code (3) is not valuable enough for us to be jumping through hoops in both solutions.

7 seems hard after proguarding.

It still seems to me like achieving 1,2,4,5 will be great value and we can do this without UI, by publishing intents to apps and getting back intents from apps.

If we decide to prioritize other goals, we should talk to the Android Frameworks team and get them to explicitly sign off on their design choices that we are explicitly overriding.

@google-oss-bot
Copy link
Contributor

@allisonbm92: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
connected-check-changed da1ac2e link /test connected-check-changed

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@allisonbm92 allisonbm92 merged commit 13df083 into master Apr 2, 2019
@firebase firebase locked and limited conversation to collaborators Oct 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes Override cla size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants