Skip to content
This repository has been archived by the owner on Jun 13, 2024. It is now read-only.

Set up a process to roll Gallery version in Flutter Devicelab tests #254

Open
pennzht opened this issue Aug 5, 2020 · 8 comments
Open
Assignees
Labels
enhancement New feature or request P3 Priority 3 - Could have

Comments

@pennzht
Copy link
Member

pennzht commented Aug 5, 2020

Is your feature request related to a problem? Please describe.
(Please add)

Describe the solution you'd like
It would be great to have a process to roll the Gallery version regularly in Devicelab Tests in Flutter.

Describe alternatives you've considered
(Please add)

Additional context
Please see flutter/flutter#62986 for more discussion.

@pennzht pennzht added the enhancement New feature or request label Aug 5, 2020
@jmagman
Copy link
Member

jmagman commented Aug 5, 2020

Copying discussion from flutter/flutter#62986

Is there any way we can move these tests to customer tests https://github.com/flutter/tests/blob/master/registry/flutter_gallery.test and out of Flutter proper? That is also pinning a SHA that already needs to roll.

https://github.com/flutter/tests#flutter-tests

This repository contains references to tests that are run with every commit to Flutter to verify that no breaking changes have been introduced. The tests referenced by this repository are typically maintained by people outside of the Flutter team, as part of the development of their applications. They are intended to give the Flutter team visibility into how their changes affect real-world developers using Flutter.

flutter_gallery_v2_chrome_run_test particularly looks look a good candidate. flutter_gallery_v2_web_compile_test is gathering benchmarks for compilation times, and I don't think there's a concept of benchmark tracking in customer_testing. However we should weigh the value of tracking compilation times for the new gallery (versus any of the other integration tests we already have in the flutter repo and not in a different repo) versus the complexity of maintaining this new "roll".

\cc @johnsonmh @chunhtai @Hixie

@jmagman
Copy link
Member

jmagman commented Aug 5, 2020

There's also duplicate flutter/flutter#63004.

@christopherfujino
Copy link
Member

i'll close mine, thanks for this @pennzht

stuartmorgan pushed a commit to stuartmorgan/gallery that referenced this issue Aug 13, 2020
* Add debugLabel to HighlightFocus

* Remove decrated property

* Add focus to DestinationCards

* Add index property to Forms

* Move BackLayer stuff to its own file

* Add focus

* Revert deprecated change

* Address feedback


Former-commit-id: 49c7244
@xster
Copy link
Member

xster commented Sep 3, 2020

Echoing @jmagman's suggestion.

The other ramification is that having a benchmark test driving the gallery in Flutter repo (probably unintentionally) actually makes Flutter Gallery the one true source of all dependency and transitive dependencies of all flutter/flutter repo. Since flutter/flutter enforces a single resolved version for all dependencies, and since the web macrobenchmark depends on a Flutter Gallery's git hash commit, that Flutter Gallery's pubspec.yaml effectively becomes the locked dependency version that all dependencies in flutter/flutter have to use.

This 1) creates resolution issues that might arise when changing pubspec in Flutter Gallery that will have bigger downstream ramifications when rolling and 2) changes the order of package upgrades in flutter/flutter. The sequencing will have to then be send a PR to flutter/gallery to update pubspec in order to be able to change the dependency in flutter/flutter.

@Hixie
Copy link
Contributor

Hixie commented Sep 3, 2020

I think we should seriously consider not testing or benchmarking the "real" gallery as part of Flutter's regular integration tests. Things we want to have be part of our integration tests should be in the main repo. If we want to make sure we don't break it, we could add it to the customer_testing shard (flutter/tests).

@jmagman
Copy link
Member

jmagman commented Sep 3, 2020

@Hixie Parts of it already are in the customer_testing shard. Check out my comment above about what should be easy to move and what would be trickier.

@Hixie
Copy link
Contributor

Hixie commented Sep 3, 2020

Excellent. I think it's fine for us to remove the other testing we have right now. The cost is disproportionate to the benefit and we are so deep in debt here that the benefit of reducing the cost would offset even a non-trivial benefit.

@yjbanov
Copy link
Contributor

yjbanov commented Sep 15, 2020

@Hixie what is it about the customer_testing shard that would make it easier to maintain? I'm curious if we could apply the same method in the benchmark. This way we would still be able to benchmark the gallery without the drawbacks of whatever it is we're doing currently.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request P3 Priority 3 - Could have
Projects
None yet
Development

No branches or pull requests

7 participants