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
firebase-test-labs 0.0.2 #4077
base: master
Are you sure you want to change the base?
firebase-test-labs 0.0.2 #4077
Conversation
Don't forget to follow semantic versioning! Breaking changes (including step inputs) should be in major versions only.
|
Let me know of any questions, suggestions, or action items I need to complete. Thank you! |
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.
Hello @AnthonyDadeWT 👋
A general observation before doing a deeper review of this step: it's probably not a good idea to do both the Flutter build and the Firebase testing in a single step. There are existing Flutter steps for building iOS or Android artifacts, this step should probably work with the outputs of the build step.
It's easier to debug build issues when one step is doing one thing only. Also, this step could be useful for non-Flutter projects too, if it was working with standard app artifacts.
Hello, and thank you for your review!
This is a good point. I did implement this for Android, on line 101 of my step.sh starts the logic of checking whether or not we need to build an APK based on if $BITRISE_APK_PATH exists. However I don't think this would actually be possible on the iOS side, because according to flutter's documentation on deploying integration tests to FTL , you must first run Now that I think about it, I guess users would be able to pass this in as an additional argument to the Flutter Build step. Although it is also required to run
This is a thought I had as well since this Step should be compatible. However, I was afraid that since Bitrise has "Device Testing" Steps for iOS and Android already, I feared that it would be considered as a duplicate step. Although I will say there are major differences from the Device Testing Steps and the one I've created here, where this Step uses the user's Firebase project, runs the tests asynchronously, and the results are displayed there on Firebase instead of Bitrise. |
I see, thank you for the extra context and details :) In this case, I'm thinking maybe the best compromise would be to add "Flutter" to the step title to avoid confusion and differentiate it from other Firebase steps. What do you think about this compromise? |
Yes that makes sense to me. I thought I couldn't have a platform's name in the title due to the rules on Steps but I see other Steps have Platforms in the name. Here is my plan for updating this step to follow your guidance. Change to accept outputs from Flutter Build (for a release build IPA) Then modify the Step to accept these outputs and only run the execution of sending these items to FTL. Update documentation to guide user to use these Steps before this Step in a workflow. Change name to "Flutter Device Testing" ? - open to suggestions here Let me know how that all sounds. It will take me some time to get this done. |
@AnthonyDadeWT the plan looks good to me, thanks for outlining this! Naming: I would keep "Firebase" in the title because of your previous point that it does something similar to the first-party step, but with the user's Firebase account. So anyone who wants to set up Firebase testing in a custom way would find your step. |
What to do if the build fails?
At the moment contributors do not have access to the CI workflow triggered by StepLib PRs. In case of a failed build, we ask for your patience. Maintainers of Bitrise Steplib will sort it out for you or inform you if any further action is needed.
New Pull Request Checklist
Please mark the points which you did / accept.
bitrise run test
(in the step's repository)bitrise run audit-this-step
(in the step's repository - note, if you don't have this workflow in yourbitrise.yml
, you can copy it from the step template.)New Step
Thank you for the new Step share! The CI check might will fail due to our extended validation engine. Nothing to worry about yet, we will get back to you shortly.