Reduce usage of context getters and globals in the flutter tool

@zanderso

Internal: b/139713315

The flutter_tool currently makes heavy usage of context getter globals to inject objects throughout the tool for both configuration and testability issues. For example:

Foo get foo => context.get<Foo>();

class Bar {

  void doThing() {
    foo.fizz(); // context global
  }
}

This is making it more difficult than necessary to unit test tooling code. Besides mocking foo, we must also ensure the context injection is correct which leads to a significant amount of boilerplate configuration for each test case. Mistakes in this boilerplate can lead to tests silently doing the wrong thing.

Previously I attempted to clean this up with the Testbed class, but this hasn't succeeded it simplifying the test cases.

Instead, we should gradually reduce our usage of these context getters which removes the need for the test configuration and makes it more explicit which instances are being used for tests. For example, rewriting the snippet above becomes:

class Bar {
  Bar({Foo foo}) : _foo = foo;

  final Foo _foo;

  void doThing() {
    _foo.fizz();
  }
}

To ensure we don't regress in usage of context getters, we can use the forthcoming testWithoutContext test method that throws if context.get is invoked. See also #45739

cc @zanderso

Sorry for the late discussion. Just saw this now.

This is making it more difficult than necessary to unit test tooling code. Besides mocking foo, we must also ensure the context injection is correct which leads to a significant amount of boilerplate configuration for each test case. Mistakes in this boilerplate can lead to tests silently doing the wrong thing.

Doesn't this describe dependency injection in general? I would expect dependency injection to make unit testing easier rather than harder. If an implicit dependency supplier is the identified issue (I have trouble following through the upstream discussions), should we just solve that? e.g. no dependencies are fulfilled through the context unless you set them up. Then we can still avoid multilayer construction injection which forces us to leak knowledge of descendent dependencies in classes just to pass injections around.

@christopherfujino

You are of course perfectly free to disagree, but this was the decision that myself, @christopherfujino , @jmagman , and @zanderso came to several months ago.

we can still avoid multilayer construction injection which forces us to leak knowledge of descendent dependencies in classes just to pass injections around.

You don't need to leak implementation details if you inject the classes you need instead of constructing them, but unwinding the context injection requires working from the bottom up, so in the interim there are some cases where we inject interfaces just to construct other types.

[flutter_tools] remove globals from desktop configuration (#67146)

[flutter_tools] refactor engine locator to its own class (#67274)

[flutter_tools] add more docs to cocoapods, move to globals (#67237)

[flutter_tools] remove globals from FlutterValidator, add documentati…

[flutter_tools] remove globals from compile and devices (#67485)

[flutter_tools] remove globals from IntelliJ validator, refactor test…