-
Notifications
You must be signed in to change notification settings - Fork 624
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
Conversation
This commit adds a new smoke test for Firestore and some infrastructure for the other smoke tests that be added soon.
smoke-tests/src/androidTest/java/com/google/firebase/testing/common/TestApkUtil.java
Outdated
Show resolved
Hide resolved
...tests/src/androidTestFirestore/java/com/google/firebase/testing/firestore/FirestoreTest.java
Outdated
Show resolved
Hide resolved
...tests/src/androidTestFirestore/java/com/google/firebase/testing/firestore/FirestoreTest.java
Outdated
Show resolved
Hide resolved
...tests/src/androidTestFirestore/java/com/google/firebase/testing/firestore/FirestoreTest.java
Outdated
Show resolved
Hide resolved
smoke-tests/src/androidTest/java/com/google/firebase/testing/common/SmokeTestBase.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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
- Test the release infrastructure by testing released artifacts- proguard.txt, desugar.
- Test compatibility across SDKs and API boundaries.
- Have test code and testable code sit side by side.
- Run test code and testable code in test and app threads respectively.
- Package test code and testable code into test and app APK respectively.
- Test our products along with AGP's app extension (as opposed to library extension)
- 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
- Product teams are required to learn the channel APIs.
- Nit: Channel APIs were fluent when it came to our tasks based APIs. But not otherwise.
Line 39 in 370f2a5
FirebaseAuth auth = FirebaseAuth.getInstance(); - In theory, these APIs would have to be evolved with the underlying APIs of the products.
- The design goal of having channel APIs report the underlying SDK error instead of (say) "View could not be fond on screen" might be quite hard once we start proguarding.
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.
smoke-tests/src/main/java/com/google/firebase/testing/common/Tasks2.java
Show resolved
Hide resolved
smoke-tests/src/firestore/java/com/google/firebase/testing/firestore/FirestoreActivity.java
Outdated
Show resolved
Hide resolved
@allisonbm92: The following test failed, say
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. |
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.