It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@amirh can these be added to customer_testing? Cloning repos at HEAD means that the framework CI builds are no longer reproducible (they are dependent on another repo's HEAD), and will break release branches. See go/why-flutter-releases-break for more context.

@amirh second what @christopherfujino says. #69309 title also indicates it should live there Analyze flutter/plugins as part of flutter/tests. Can we get context as to why this was added to the framework instead?

This was originally proposed to be added as part of customer_testing but the feedback was to add it as a separate test, see: flutter/tests#66 (comment).

Good point about it not being predictable when cloning at HEAD, my reluctancy to pin to a specific commit is that we'll have to setup something like an auto-roller to continuously increment these commit hashes. My intuition is that this is unlikely to get broken due to changes to flutter/plugins as it runs the same tests against flutter/master on presubmit. I'd suggest we monitor and if we see that this flaked we put in the time to add a roller and change to a fixed commit. WDYT?

This was originally proposed to be added as part of customer_testing but the feedback was to add it as a separate test, see: flutter/tests#66 (comment).

Good point about it not being predictable when cloning at HEAD, my reluctancy to pin to a specific commit is that we'll have to setup something like an auto-roller to continuously increment these commit hashes. My intuition is that this is unlikely to get broken due to changes to flutter/plugins as it runs the same tests against flutter/master on presubmit. I'd suggest we monitor and if we see that this flaked we put in the time to add a roller and change to a fixed commit. WDYT?

@amirh what about release branches?

We should use the same mechanism we use for engine->framework tests: fetch the newest version of the plugins repo that is not newer than the framework commit. In case of branches, it should pick the version of the framework at the branch point, not the latest cherry-pick.

We should use the same mechanism we use for engine->framework tests: fetch the newest version of the plugins repo that is not newer than the framework commit. In case of branches, it should pick the version of the framework at the branch point, not the latest cherry-pick.

yeah, this would work.

Do you happen to know which engine->framework test is doing this already?

this is the script that is used https://github.com/flutter/engine/blob/master/tools/clone_flutter.sh. this is hard-coded for engine use, I created this issue to write a general-purpose tool to use across our repos: #57062

flutter/tests#66 (comment)

I'm suggesting that the plugins tests have nothing to do with flutter/tests or customer_testing at all.

Should we be treating "first party" flutter/plugins as a special though? Does flutterfire need a shard? It would be nice if instead there is a mechanism for any plugin author to "apply" test against commits of flutter, similar to customer_testing but upstream instead of downstream.

@jmagman can you elaborate on downstream vs upstream?

I agree that it would be great to have a mechanism to let any package author be part of our test-suite, which is the purpose of flutter/tests and the reason this was initially added there. Though it is impractical from resources point of view to test all the plugins in the ecosystem (resources are the reason flutter/tests#66 was rejected and this was not even running most of the flutter/plugins test suite).

The flutter/plugins repository has been taking the productivity hits of testing against master in order to protect the rest of the ecosystem, and I think that for almost all of the recent Flutter releases there were multiple regressions avoided as they were detected early on flutter/plugins master test. Given that, I do think flutter/plugins justifies special treatment in terms of resource allocation, whether that is through a dedicated test shard, or through extra customer_testing budget (which was the original proposal).

@jmagman can you elaborate on downstream vs upstream?

Sorry, that wasn't clear at all. I meant it would be awesome if flutter app customers and plugin authors could both make use of customer_testing instead of tightly coupling any of them into flutter/flutter.

Though it is impractical from resources point of view to test all the plugins in the ecosystem

Sure, I wasn't really proposing that, any more than customer_test tests all flutter apps in existence.

or through extra customer_testing budget (which was the original proposal).

That's my vote (resurrecting some form of flutter/tests#66). I don't think flutter/plugins should have its own shard any more than flutterfire or flutter/gallery should, which also catch flutter/flutter regressions. See #57062 flutter/gallery#254, go/why-flutter-releases-break, etc

@jmagman what are the arguments against running flutter/plugins in its own test shard? the point about indeterministic builds is valid and we should do one of the solutions proposed above (side note: my intuition is that it's pretty unlikely to flake, even not on release builds as flutter/plugins presubmits are running these tests against flutter master and stable).

Are there more cons to for adding another shard for it?

Hixie's point about keeping customer_testing fast and simple is a valid reason to exclude it from flutter/tests (and I hope to add even more of the flutter/plugins test suite which will require more resources).

I'm not sure "not giving flutter/plugins a special treatment in flutter/flutter CI" should be a goal on it's own. The flutter/plugins repository is special in that it is maintained by the flutter team, and we should set things up in a way that makes the team most productive.

I'm fine with us adding plugins tests to flutter/tests, provided that they fit within the constraints. But the constraints on what we would accept from the general populace are much tighter than what we would accept from people working directly on Flutter since our expectations from people on the team are much higher (e.g. we would expect team members to work on fixing flakes while we just wouldn't take the risk of flakiness from contributed tests we don't directly control).

I'm not sure "not giving flutter/plugins a special treatment in flutter/flutter CI" should be a goal on it's own.

I'm not trying to be pedantic, I just want clear criteria and processes for what repos are now essentially rolled into the flutter/fluter testing (even with the newest-commit-older-than-framework tricks, it's still essentially a roll). It always causes the people keeping the tree open (gardener now) and release engineers headaches. They now need to understand the semantics of the plugins repo, when the plugin tree goes red post-submit unrelated to a flutter/flutter commit, it could also close the flutter/flutter tree. We keep adding new repos, new dependencies, new ways to clone/download/determine versions. I guess we can just try it and see.

I'm not trying to be pedantic, I just want clear criteria and processes for what repos are now essentially rolled into the flutter/fluter testing (even with the newest-commit-older-than-framework tricks, it's still essentially a roll). It always causes the people keeping the tree open (gardener now) and release engineers headaches. They now need to understand the semantics of the plugins repo, when the plugin tree goes red post-submit unrelated to a flutter/flutter commit, it could also close the flutter/flutter tree. We keep adding new repos, new dependencies, new ways to clone/download/determine versions.

Yeah, to this point, the main reason I would want to use customer_testing is because it's an established, known framework for depending on external repositories that has a documented procedure. This provides the engine sheriff/framework gardener/release engineer a simple decision tree for handling failures. I would be happy to duplicate this framework, and call it the non_customer_slow_testing shard.

@jmagman fair point about the newest-commit-older-than-framework thing would mean auto roller with no testing, I'm going to pin to a specific commit, will do "manual rolls" for now, hopefully one day we'll set up an auto-roller.

@christopherfujino The customer_testing code is all about dealing with a registry of multiple repositories and sharding test execution of the different repositories. I don't see a reason to do all that for a shard that only tests flutter/plugins.

If we pin we should either have an automated process in place like the autorollers, or at a very minimum a documented, scheduled process in place so that we know when and how it will be updated. We've suffered greatly in the past with manual undocumented ad hoc processes. (For example, the lack of automation around the pubspec.yaml dependencies is a source of trouble.)

Good point, I won't make the test blocking before we have a process/mechanism for regular version updates.