Pin gallery version #62986

pennzht · 2020-08-05T15:21:26Z

Description

Pin the version of flutter/gallery in devicelab tests in flutter/flutter.

Related Issues

Tests

No tests were added.

Checklist

Before you create this PR, confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I signed the CLA.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I updated/added relevant documentation (doc comments with ///).
All existing and new tests are passing.
- I only ran the tests flutter_gallery_v2_chrome_run_test and flutter_gallery_v2_web_compile_test, and they are passing.
The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
- The following error and info were raised, but they are unrelated to the changes in this PR.


  error • A value of type 'Future<Dwds> Function({AssetReader assetReader, Stream<BuildResult> buildResults, Future<ChromeConnection> Function() chromeConnection,
         bool enableDebugExtension, bool enableDebugging, ExpressionCompiler expressionCompiler, String hostname, LoadStrategy loadStrategy, void Function(Level,
         String) logWriter, bool restoreBreakpoints, bool serveDevTools, Future<String> Function(String) urlEncoder, bool useSseForDebugProxy, bool verbose})' can't
         be assigned to a variable of type 'Future<Dwds> Function({AssetReader assetReader, Stream<BuildResult> buildResults, Future<ChromeConnection> Function()
         chromeConnection, bool enableDebugExtension, bool enableDebugging, ExpressionCompiler expressionCompiler, String hostname, LoadStrategy loadStrategy, void
         Function(Level, String) logWriter, bool serveDevTools, Future<String> Function(String) urlEncoder, bool useFileProvider, bool useSseForDebugBackend, bool
         useSseForDebugProxy, bool verbose})' • ../../packages/flutter_tools/lib/src/build_runner/devfs_web.dart:152:33 • invalid_assignment
   info • The method doesn't override an inherited method • ../../packages/flutter_tools/lib/src/build_runner/devfs_web.dart:603:18 •
          override_on_non_overriding_member

2 issues found. (ran in 58.2s)

I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read Handling breaking changes.

No, no existing tests failed, so this is not a breaking change.
- I only ran the tests flutter_gallery_v2_chrome_run_test and flutter_gallery_v2_web_compile_test, and they are passing.

christopherfujino · 2020-08-05T15:46:31Z

dev/devicelab/lib/versions/gallery.dart

+// found in the LICENSE file.
+
+/// The pinned version of flutter gallery, used for devicelab tests.
+const String galleryVersion = 'a208eac6e6e8336ae9820e54c572c099231f1da2';


This is awesome. Your PR LGTM.

My only question is if there is an official documented procedure for rolling the gallery here, and if it can be updated to reference this file.

I don't know. I'm asking right now.

+1, interested in how this works as gallery changes - don't we want master to test against the newest gallery? this seems more reasonable for branches than master (but I may be misunderstanding)

If we always ask flutter/flutter:master to clone flutter/gallery:master, an error in Gallery might cause Flutter's build to fail. These tests always uses latest flutter/flutter:master.
I think the best way is to use a commit hash instead of a branch, and set up a process to roll it regularly. (But this is a future task.)

@christopherfujino @pcsosinski

There isn't a process to roll the gallery version yet. It would be a good idea to add this process, some time in the future.

christopherfujino · 2020-08-05T15:54:33Z

I can confirm these pass for me locally

guidezpl

  _    ___ _____ __  __ 
 | |  / __|_   _|  \/  |
 | |_| (_ | | | | |\/| |
 |____\___| |_| |_|  |_|

pcsosinski

Would like to see some process around how we update this hash (who is responsible, how often, etc) to make sure we don't get stuck on an old version. thanks!

christopherfujino · 2020-08-05T17:42:02Z

Would like to see some process around how we update this hash (who is responsible, how often, etc) to make sure we don't get stuck on an old version. thanks!

Agreed, I think establishing a roll process for the gallery should be a high priority. I recall at least once the Flutter framework build was broken because of a change that had already landed in the gallery repo, but hadn't been rolled.

jmagman · 2020-08-05T17:56:01Z

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.

pennzht · 2020-08-05T18:02:54Z

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.

What are the differences between placing tests here vs. placing tests in https://github.com/flutter/tests/blob/master/registry/flutter_gallery.test?

christopherfujino · 2020-08-05T18:06:55Z

What are the differences between placing tests here vs. placing tests in https://github.com/flutter/tests/blob/master/registry/flutter_gallery.test?

customer_testing runs on either cirrus or LUCI, whereas this test runs on the devicelab (actual hardware).

jmagman · 2020-08-05T18:10:43Z

What are the differences between placing tests here vs. placing tests in https://github.com/flutter/tests/blob/master/registry/flutter_gallery.test?

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".

christopherfujino · 2020-08-05T18:18:23Z

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 way 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".

We also have gallery v1 benchmarks

pennzht added 6 commits August 5, 2020 16:51

Add gallery version.

5a237dd

Use new gallery cloning.

9519c76

Unify.

1487119

Revert two other changes.

15c222f

Use getNewGallery and pinned version.

52a340e

Use show galleryVersion.

09ca101

flutter-dashboard bot added the c: contributor-productivity Team-specific productivity, code health, technical debt. label Aug 5, 2020

Use correct directory.

cec29ee

pennzht requested review from christopherfujino, guidezpl and pcsosinski August 5, 2020 15:38

Remove trailing blank line.

1a39e49

christopherfujino approved these changes Aug 5, 2020

View reviewed changes

guidezpl approved these changes Aug 5, 2020

View reviewed changes

pcsosinski approved these changes Aug 5, 2020

View reviewed changes

pennzht added the waiting for tree to go green label Aug 5, 2020

pennzht mentioned this pull request Aug 5, 2020

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

Open

christopherfujino mentioned this pull request Aug 5, 2020

Feature request: official procedure for rolling the Flutter Gallery into the Framework #63004

Closed

fluttergithubbot merged commit 504963b into flutter:master Aug 5, 2020

christopherfujino mentioned this pull request Sep 2, 2020

Flutter stable framework 1.20.3 cherrypicks #64984

Merged

mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020

Pin gallery version (flutter#62986)

249d86e

christopherfujino mentioned this pull request Sep 14, 2020

Flutter Stable Framework Cherrypicks 1.20.4 #65787

Merged

github-actions bot locked as resolved and limited conversation to collaborators Aug 5, 2021

Pin gallery version #62986

Pin gallery version #62986

Uh oh!

Conversation

pennzht commented Aug 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Tests

Checklist

Breaking Change

Uh oh!

christopherfujino Aug 5, 2020

Choose a reason for hiding this comment

Uh oh!

pennzht Aug 5, 2020

Choose a reason for hiding this comment

Uh oh!

pcsosinski Aug 5, 2020

Choose a reason for hiding this comment

Uh oh!

pennzht Aug 5, 2020

Choose a reason for hiding this comment

Uh oh!

pennzht Aug 5, 2020

Choose a reason for hiding this comment

Uh oh!

christopherfujino commented Aug 5, 2020

Uh oh!

guidezpl left a comment

Choose a reason for hiding this comment

Uh oh!

pcsosinski left a comment

Choose a reason for hiding this comment

Uh oh!

christopherfujino commented Aug 5, 2020

Uh oh!

jmagman commented Aug 5, 2020

Uh oh!

pennzht commented Aug 5, 2020

Uh oh!

christopherfujino commented Aug 5, 2020

Uh oh!

jmagman commented Aug 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

christopherfujino commented Aug 5, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

pennzht commented Aug 5, 2020 •

edited

Loading

jmagman commented Aug 5, 2020 •

edited

Loading