Skip to content

Conversation

@gmackall
Copy link
Member

@gmackall gmackall commented Apr 8, 2024

Removes TestUtils.java, in favor of using @VisibileForTesting instead. I had to change a couple of types from the interface type to the actual implementation type too, to accomplish the removal.

fixes flutter/flutter#146457

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@gmackall gmackall marked this pull request as ready for review April 8, 2024 22:47
@gmackall gmackall requested a review from camsim99 as a code owner April 8, 2024 22:47
@gmackall gmackall requested review from a team and matanlurey April 8, 2024 22:47
Copy link
Contributor

@matanlurey matanlurey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks... better :)

We should chat in person sometime about what the testing strategy here is, like it's not common for me to see so many internal-state fields changed/probed during tests, but I also might be missing some bigger picture.

LGTM!

@gmackall
Copy link
Member Author

gmackall commented Apr 8, 2024

Looks... better :)

We should chat in person sometime about what the testing strategy here is, like it's not common for me to see so many internal-state fields changed/probed during tests, but I also might be missing some bigger picture.

LGTM!

I agree, but I think fixing the testing strategy for the camera_android implementation of the camera plugin would be relatively low priority for our team.

It is hard to justify spending much effort on it, when that effort could instead be spent improving the new CameraX implementation that we would ideally get everyone to switch to. Happy to talk about it either way though

@matanlurey
Copy link
Contributor

Yeah, maybe the take away will just be "what not to do in camerax".

@gmackall gmackall added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 8, 2024
@auto-submit auto-submit bot merged commit d4cd4f0 into flutter:main Apr 8, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 9, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Apr 9, 2024
flutter/packages@6b4d8b6...17f55d3

2024-04-09 barpac02@gmail.com Release compileSdk changes (flutter/packages#6491)
2024-04-08 34871572+gmackall@users.noreply.github.com [camera_android] Remove `TestUtils.java` (flutter/packages#6490)
2024-04-08 engine-flutter-autoroll@skia.org Roll Flutter from 98d23f7 to 533d04d (12 revisions) (flutter/packages#6488)
2024-04-08 32538273+ValentinVignal@users.noreply.github.com [go_router_builder] Add `restorationScopeId` to `ShellRouteData` (flutter/packages#6238)
2024-04-08 39844581+kforjan@users.noreply.github.com [go_router] Fixes deep links with no path (flutter/packages#6447)
2024-04-08 stuartmorgan@google.com [in_app_purchase] Convert Android data objects to Pigeon (flutter/packages#6453)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
gilnobrega pushed a commit to gilnobrega/flutter that referenced this pull request Apr 22, 2024
flutter/packages@6b4d8b6...17f55d3

2024-04-09 barpac02@gmail.com Release compileSdk changes (flutter/packages#6491)
2024-04-08 34871572+gmackall@users.noreply.github.com [camera_android] Remove `TestUtils.java` (flutter/packages#6490)
2024-04-08 engine-flutter-autoroll@skia.org Roll Flutter from 98d23f7 to 533d04d (12 revisions) (flutter/packages#6488)
2024-04-08 32538273+ValentinVignal@users.noreply.github.com [go_router_builder] Add `restorationScopeId` to `ShellRouteData` (flutter/packages#6238)
2024-04-08 39844581+kforjan@users.noreply.github.com [go_router] Fixes deep links with no path (flutter/packages#6447)
2024-04-08 stuartmorgan@google.com [in_app_purchase] Convert Android data objects to Pigeon (flutter/packages#6453)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
TecHaxter pushed a commit to TecHaxter/flutter_packages that referenced this pull request May 22, 2024
Removes `TestUtils.java`, in favor of using `@VisibileForTesting` instead. I had to change a couple of types from the interface type to the actual implementation type too, to accomplish the removal.

fixes flutter/flutter#146457
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App p: camera platform-android

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TestUtils (flutter/packages) should be deleted/tests refactored

2 participants