Avoid pinning mockito to single version

[natebosch]

The pinned version of mockito is incompatible with the latest released version of test_api, and therefore test. With flutter package pinning, the release of the flutter SDK that updates to the next version of test_api will break the version solve with these packages. Using a constraint that allows a range of versions of mockito should allow a valid version solve.

This is required to unblock https://github.com/flutter/flutter/pull/123350

[stuartmorgan-g]

Mockito is pinned deliberately (which is why this is failing repo checks); this regresses https://github.com/flutter/packages/pull/3466. See also https://github.com/flutter/flutter/issues/108555, which is an OOB breakage affecting multiple trees that was caused by not pinning mockito.

What version of mockito is compatible with the update? Can we just roll the pin?

[natebosch]

Mockito version 5.4.0 will be compatible.

It is currently not published because it was retracted by @zanderso , but when it is published it should be possible to upgrade to it.

Mockito is pinned deliberately

That's a semver anti-pattern and you should expect to not be cross-compatible with different versions of flutter_test in some cases. We will need to find some way to commit changes like https://github.com/flutter/flutter/pull/123350 with a red CI and then update the pin or something similar if we insist on pinning.

Package pinning is, in my opinion, an anti-solution to the problem, which is that we are misusing deprecated diagnostics.

[stuartmorgan-g]

We will need to find some way to commit changes like flutter/flutter#123350 with a red CI and then update the pin or something similar if we insist on pinning.

I don't see why we couldn't just update flutter/packages with the new version of mockito first, then roll flutter/flutter, without anything having to be landed on red.

which is that we are misusing deprecated diagnostics.

How so?

[natebosch]

How so?

The reason to deprecate an API instead of removing it is so that there is a release which is non-breaking for consumers and gives them a heads up. Treating the the release as if it was breaking instead eliminates the value of the deprecation.

I don't see why we couldn't just update flutter/packages with the new version of mockito first, then roll flutter/flutter, without anything having to be landed on red.

There is no guarantee that we can make mockito forwards compatible in every case. It should work in this case.

[natebosch]

FWIW I think we misuse deprecations in other places too - flutter isn't the only one - but @lrhn and do get blocked from informing our users early about deprecations because of "strict" CI like this.

[stuartmorgan-g]

The reason to deprecate an API instead of removing it is so that there is a release which is non-breaking for consumers and gives them a heads up.

I understand the concept of deprecation periods. What is the mechanism by which you propose that the Flutter team receive that heads up without surfacing it in CI?

Treating the the release as if it was breaking instead eliminates the value of the deprecation.

You are overloading the term "breaking" here though. There's a vast difference between breaking our CI and breaking clients of our package. Not breaking clients has enormous value.

I don't see why we couldn't just update flutter/packages with the new version of mockito first, then roll flutter/flutter, without anything having to be landed on red.

There is no guarantee that we can make mockito forwards compatible in every case. It should work in this case.

We can adjust tooling to allow non-carat ranges instead of only exact pins, such that we can set the upper range to a known-working, already-released version.

[natebosch]

You are overloading the term "breaking" here though.

I'm using it the way that I see it when this issue surfaces.

"This is breaking the flutter SDK, you must roll this back".

I understand the concept of deprecation periods. What is the mechanism by which you propose that the Flutter team receive that heads up without surfacing it in CI?

We need some warning level that provides feedback but isn't an error. I don't know the best way to surface that through our various CI.

[natebosch]

We can adjust tooling to allow non-carat ranges instead of only exact pins, such that we can set the upper range to a known-working, already-released version.

I think that should work. It makes the critical path longer, and it increases the general friction around flutter's pinned packages.

[stuartmorgan-g]

We need some warning level that provides feedback but isn't an error. I don't know the best way to surface that through our various CI.

I'm not aware of a mechanism to do that.

It makes the critical path longer, and it increases the general friction around flutter's pinned packages.

Compared to what? The other options in aware of that are available to us are:

• Don't pin, keep deprecation warnings. This leads to OOB mutli-tree breakage events like the one I liked to, which are very high friction.
• Don't pin, disable deprecation warnings. This means we won't find out about deprecation warnings in any package we use. I'm skeptical that never finding out about deprecation, until we need to upgrade for some reason and suddenly discover a backlog of deprecated API use that we must immediately fix, will decrease friction overall. (I assume given your comments above that you would at least partially agree, because avoiding that is the whole point of deprecation warnings.)

I'm open to suggestions if you have them. I didn't choose the current approach for this repo because I think it's ideal, but because it seemed like the best available option.

[natebosch]

Compared to what?

Compared to not needing a PR in flutter/packages before we can roll the test packages in flutter/flutter.

[stuartmorgan-g]

Compared to what?

Compared to not needing a PR in flutter/packages before we can roll the test packages in flutter/flutter.

That's an outcome, not an approach. I've explained the approaches that I am aware of for achieving that specific outcome, and the corresponding friction they create in other ways.

[Hixie]

test-exempt: dependency roll

[natebosch]

@stuartmorgan - these changes only impact local development. From my reading of https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#version-and-changelog-updates this change should not change versions or add ## NEXT entries.

Should I add the label to override the version change check?

[stuartmorgan-g]

That's correct for the changes so far. It looks like mockito 5.4.0 requires Flutter 3.3 though, which will require SDK version bumps, and those will need NEXT entries. (I can take care of that for you with our repo tools right now.)

[stuartmorgan-g]

Overriding version: mockito changes are dev-only, and SDK changes we deliberately don't do releases for since they are purely limiting.

[natebosch]

doesn't seem to update the mock files

I found some extends Mock and then stopped looking - I should have checked more directories. I've regenerated the mocks in the packages that were using codegen.

[ditman]

Actually I tried to update some of the mocks that were generated with 5.3.2 on this branch, and they stay the same, so I guess the examples are fine too!

dit@dit:/work/flutter/packages/packages/google_maps_flutter/google_maps_flutter_web/example$ cat integration_test/google_maps_controller_test.mocks.dart 
// Mocks generated by Mockito 5.3.2 from annotations
// in google_maps_flutter_web_integration_tests/integration_test/google_maps_controller_test.dart.
// Do not manually edit this file.

[natebosch]

and they stay the same,

Hmm, I would expect at least the // Mocks generated by Mockito 5.3.2 to change...

[natebosch]

I added some generate_for config and regenerated mocks in packages using a directory named integraion_test/ instead of test/. I also removed a stale mock file that was no longer used at all.

[natebosch]

~~https://cirrus-ci.com/task/5467466374053888~~

The following packages had errors:
  camera_android_camerax
See above for full details.

~~Searching the page for camera_android_camerax does not find any other occurrences...~~ ~~Is there some other way to see the logs for what failed?~~ flutter test passes for me in packages/camera/camera_android_camerax/

Edit: I found the link to the full log: https://api.cirrus-ci.com/v1/task/5467466374053888/logs/unit_test.log

[ditman]

submit-queue is just the check that stops merges to main if there's currently a broken test in the top of the repo. This PR looks ready to land.

Thanks for digging in the unchanged mocks! @natebosch