Skip to content
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

Support skipping fresh install #142

Closed
mcoolidge opened this issue Oct 18, 2019 · 15 comments · Fixed by #144
Closed

Support skipping fresh install #142

mcoolidge opened this issue Oct 18, 2019 · 15 comments · Fixed by #144
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@mcoolidge
Copy link

Flutter added a parameter: "flutter drive --no-build" to allow running on a preinstalled app and not re-building/installing. This would be useful for a number of reasons including working around native permission dialog blockers.

flutter/flutter#30818

@mmcc007
Copy link
Owner

mmcc007 commented Oct 18, 2019

Ironically, the '--no-build' option was created as a result of an issue I opened (flutter/flutter#28662) for another project. 😄

I usually like to describe the use case(s) adding a feature covers and how it should be used. In this case it seems that there is a clear performance advantage in using this feature if re-running screenshots locally (when not in CI). So adding an option to enable this feature from the command line would seem to be the way to go. For example,

screenshots --no-build

Would this work for you?

Can you elaborate on any other use cases if any (for example, native permission dialog blockers)?

@mmcc007 mmcc007 added enhancement New feature or request question Further information is requested labels Oct 18, 2019
@mcoolidge
Copy link
Author

My immediate use case is to work around native iOS permission notifications. In this case, my plan is to install the app, use a command to accept the permissions, then run the screenshots tests.

We might also have backend web service changes that need to be tested against a previously installed app to make sure nothing broke. It would not be necessary to rebuild the app in this case.

@mmcc007
Copy link
Owner

mmcc007 commented Oct 19, 2019

Both use cases seem to be ones that are run locally (and not in a CI environment). So implementing by passing a parameter from the command line (as described above) seems like the way to go.

Please confirm.

@mcoolidge
Copy link
Author

These would mostly be run in a CI env but also manually as needed. The additional command line parameter would be perfect for us!! Thanks so much and it's funny how you wrote up the initial issue related to '--no-build'.

@mmcc007
Copy link
Owner

mmcc007 commented Oct 19, 2019

On reconsideration:
If passed on the command line, it would apply to all devices. For the more general use case where testing is being done on multiple devices, it might make sense to limit it to specific devices in the screenshots.yaml. (I am looking into splitting-off the core of screenshots into a more general testing platform for multiple attached devices, so this approach might be a better fit)

Would a config in screenshots.yaml still work for you?

@mcoolidge
Copy link
Author

That would work as well!

@mmcc007
Copy link
Owner

mmcc007 commented Oct 20, 2019

Implemented!
Try it out with:

git clone https://github.com/mmcc007/screenshots.git
cd screenshots
git checkout #142-no-build
pub global activate --source path .

For usage, see https://github.com/mmcc007/screenshots/blob/%23142-no-build/README.md#device-parameters.

Please let me know how it goes.

@mcoolidge
Copy link
Author

Works perfectly! Very fast and works as expected

@mmcc007
Copy link
Owner

mmcc007 commented Oct 20, 2019

Great! Thanks for confirming.

I could also add it to the command line to globally over-ride any settings in screenshots.yaml.

Please let me know if you think this would also be useful or just over-kill for your use case, or in general.

@mcoolidge
Copy link
Author

Actually, that would be useful as well so we don't need to modify our source for a specific version when doing regression testing. We consider our configs part of that "version" with the app itself.

@mmcc007
Copy link
Owner

mmcc007 commented Oct 20, 2019

Added support from command line.
Try it out with:

cd screenshots
git pull

See usage at
https://github.com/mmcc007/screenshots/blob/%23142-no-build/README.md#usage

Please let me know how it goes.

@mcoolidge
Copy link
Author

It works, but I wasn't immediately sure how to use the command. Does --[no-]build mean it could be --no-build or --build? What if you do --no-build false? Is that the same as --build true? This is due to the wording of the flutter command, but users will likely have to try each command to make sure. This might be just tweaking the docs slightly.

Maybe (and this is a different item completely) it would be helpful for screenshots to print the options it's using.

Thank you so much!! Fantastic tool!!

@mmcc007
Copy link
Owner

mmcc007 commented Oct 21, 2019

I know what you mean. Changed invocation from command line to make it consistent with how it is used in config file. Added no build messaging.

Try it out with:

cd screenshots
git pull

See usage at
https://github.com/mmcc007/screenshots/blob/%23142-no-build/README.md#usage

Please let me know how it goes.

@mcoolidge
Copy link
Author

Looks good!

@mmcc007
Copy link
Owner

mmcc007 commented Oct 22, 2019

Great!. If there is nothing else will merge and close.

Thanks for your feedback, confirmations and affirmations.

If you like screenshots, you oughta put a star🌟 on it! 😂

mmcc007 added a commit that referenced this issue Oct 22, 2019
@mmcc007 mmcc007 added good first issue Good for newcomers and removed question Further information is requested labels Oct 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants