-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Closed
Labels
P1High-priority issues at the top of the work listHigh-priority issues at the top of the work lista: tests"flutter test", flutter_test, or one of our tests"flutter test", flutter_test, or one of our testsc: contributor-productivityTeam-specific productivity, code health, technical debt.Team-specific productivity, code health, technical debt.packageflutter/packages repository. See also p: labels.flutter/packages repository. See also p: labels.team-ecosystemOwned by Ecosystem teamOwned by Ecosystem teamtriaged-ecosystemTriaged by Ecosystem teamTriaged by Ecosystem team
Description
We have a lot of complexity in flutter/packages tests around reliance on published dependencies, which can bleed into flutter/flutter due to the analysis step that's run in flutter/flutter.
We had a recent case where we ran into a new variant, which could definitely happen again. The chain of events:
- We deprecated a method in a platform interface package of a federated plugin
- As expected, the flutter/packages CI flagged that this would break analysis in the higher-level packages that make up that plugin.
- We updated the PR to include
// ignoredirectives for those cases (which would then be cleaned up in the next PR). - We landed the PR, and the change was published
- flutter/flutter started failing, because it's using a pinned version of flutter/packages, so instead of getting the
ignores immediately and the published dependency later, it got the published dependency before it picked up theignores via the flutter/packages->flutter/flutter roller.
I'm not sure yet how to best avoid this.
- We don't want do exact pinning in the package itself, because it's a published package's non-dev dependency.
- We could break things apart (like we used to) such that we update all the packages first, then deprecate the old method, but that's more overhead, and there's still no guarantee the ordering of that won't be wrong in flutter/flutter (although it would certainly be much less likely). And that doesn't work well for other similar cases that can cause analysis warnings, like adding an enum value.
- We could convert ranges to exact pins in
flutter/flutterbefore running analysis. That does mean it would sometimes be analyzing with older versions of dependencies, but that might be fine since by definition the lowest version should work. It might cause conflicts though, by preventing the resolver from working normally. - Maybe we could do something like the version-at-time script that some other combination tests use (customer tests, IIRC?), but for pub? So in flutter/flutter, for each direct dependency, we set the constraint to have a max of the newest version that had been published at the time of the pin?
Metadata
Metadata
Assignees
Labels
P1High-priority issues at the top of the work listHigh-priority issues at the top of the work lista: tests"flutter test", flutter_test, or one of our tests"flutter test", flutter_test, or one of our testsc: contributor-productivityTeam-specific productivity, code health, technical debt.Team-specific productivity, code health, technical debt.packageflutter/packages repository. See also p: labels.flutter/packages repository. See also p: labels.team-ecosystemOwned by Ecosystem teamOwned by Ecosystem teamtriaged-ecosystemTriaged by Ecosystem teamTriaged by Ecosystem team