Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
matanlurey opened this issue Apr 8, 2024 · 1 comment · Fixed by flutter/packages#6490
Closed

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

matanlurey opened this issue Apr 8, 2024 · 1 comment · Fixed by flutter/packages#6490
Labels
c: tech-debt Technical debt, code quality, testing, etc. p: camera The camera plugin package flutter/packages repository. See also p: labels. team-android Owned by Android platform team

Comments

@matanlurey
Copy link
Contributor

The camera_android plugin makes use of a TestUtils class with the following behavior:

public class TestUtils {
  public static <T> void setPrivateField(T instance, String fieldName, Object newValue) {
    try {
      Field field = instance.getClass().getDeclaredField(fieldName);
      field.setAccessible(true);
      field.set(instance, newValue);
    } catch (Exception e) {
      Assert.fail("Unable to mock private field: " + fieldName);
    }
  }

  public static <T> Object getPrivateField(T instance, String fieldName) {
    try {
      Field field = instance.getClass().getDeclaredField(fieldName);
      field.setAccessible(true);
      return field.get(instance);
    } catch (Exception e) {
      Assert.fail("Unable to mock private field: " + fieldName);
      return null;
    }
  }
}

This is ... not a good pattern. If the field needs to be made public, it should be made public (or package-private) and @VisibleForTesting used. That pattern itself should be kept to a minimum, and instead the APIs should have good inherent testability/make use of patterns like dependency injection.

This is blocking my work on flutter/packages#6461, and while I could work around it, I'd be spending a reasonable amount of time working around a bad pattern. If @reidbaker and team are on board, I'd rather spend my time helping you all delete this pattern.

@matanlurey matanlurey added p: camera The camera plugin package flutter/packages repository. See also p: labels. c: tech-debt Technical debt, code quality, testing, etc. team-android Owned by Android platform team labels Apr 8, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this issue 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
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 23, 2024
TecHaxter pushed a commit to TecHaxter/flutter_packages that referenced this issue 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
arc-yong pushed a commit to Arctuition/packages-arc that referenced this issue Jun 14, 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 subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
c: tech-debt Technical debt, code quality, testing, etc. p: camera The camera plugin package flutter/packages repository. See also p: labels. team-android Owned by Android platform team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant