felangel / mocktail

A mock library for Dart inspired by mockito
https://pub.dev/packages/mocktail
MIT License
588 stars 80 forks source link

Use debugNetworkImageHttpClientProvider and clear cache #198

Closed SimonWeidemann closed 1 year ago

SimonWeidemann commented 1 year ago

Status

READY

Breaking Changes

YES

Description

The implementation is dangers, because it will effect other tests in the test file.

For example the following app renders normally a network image and if there is an error, the error builder should be used.

/// Sample app used to showcase `mocktail_image_network`
class FakeApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: Scaffold(
        body: Center(
          child: Image.network(
            kImageUrl,
            errorBuilder: (
              context,
              error,
              stackTrace,
            ) {
              return const Text(kErrorText);
            },
          ),
        ),
      ),
    );
  }
}

/// public image url for testing
const kImageUrl = 'https://randmom-uri.com';

/// public error text for testing
const kErrorText = 'You are Doomed!';

The following test should work:

void main() {
  testWidgets('Client will return with a valid image', (tester) async {
    await mockNetworkImages(() async {
      await tester.pumpWidget(FakeApp());
      var image = find.image(const NetworkImage(kImageUrl));
      expect(image, findsOneWidget);
      await tester.pump();
      expect(image, findsOneWidget);
    });
  });

  testWidgets(
    'Client will return with failure and error widget will be shown',
    (tester) async {
      await tester.pumpWidget(FakeApp());
      expect(find.image(const NetworkImage(kImageUrl)), findsOneWidget);
      await tester.pump();
      expect(find.text(kErrorText), findsOneWidget);
    },
  );
}

Unfortunately the don't because the client will be used again. So the result of the second test will be a failure because the error text does not exist.

In the debug setting is an other way described to do so:

/// Provider from which [NetworkImage] will get its [HttpClient] in debug builds.
///
/// If this value is unset, [NetworkImage] will use its own internally-managed
/// [HttpClient].
///
/// This setting can be overridden for testing to ensure that each test receives
/// a mock client that hasn't been affected by other tests.
///
/// This value is ignored in non-debug builds.
HttpClientProvider? debugNetworkImageHttpClientProvider;

So I adjusted the code accordingly.

Still there is some issue, because already cached network images, will still be available. So I added the functionality to also delete the cache.

Type of Change

felangel commented 1 year ago

Hi @SimonWeidemann I believe this is a duplicate of the issue described in https://github.com/felangel/mocktail/issues/134#issuecomment-1550731486. See my comment regarding scoping the zone override to just the widget under test

SimonWeidemann commented 1 year ago

Hi @SimonWeidemann I believe this is a duplicate of the issue described in #134 (comment). See my comment regarding scoping the zone override to just the widget under test

I checked out your comment and adjusted the tests:

void main() {
  testWidgets('Client will return with a valid image', (tester) async {
    await mockNetworkImages(() async {
      final widget = mockNetworkImages(() => FakeApp());
      await tester.pumpWidget(widget);
      var image = find.image(const NetworkImage(kImageUrl));
      expect(image, findsOneWidget);
      await tester.pump();
      expect(image, findsOneWidget);
    });
  });

  testWidgets(
    'Client will return with failure and error widget will be shown',
    (tester) async {
      await tester.pumpWidget(FakeApp());
      debugNetworkImageHttpClientProvider = null;
      expect(find.image(const NetworkImage(kImageUrl)), findsOneWidget);
      await tester.pump();
      expect(find.text(kErrorText), findsOneWidget);
    },
  );
}

The second test still results in an error. Also when I cleaned up the caches.

I think the change would still be necessary.

The underlying code in the network image looks like this:

  // Do not access this field directly; use [_httpClient] instead.
  // We set `autoUncompress` to false to ensure that we can trust the value of
  // the `Content-Length` HTTP header. We automatically uncompress the content
  // in our call to [consolidateHttpClientResponseBytes].
  static final HttpClient _sharedHttpClient = HttpClient()..autoUncompress = false;

  static HttpClient get _httpClient {
    HttpClient client = _sharedHttpClient;
    assert(() {
      if (debugNetworkImageHttpClientProvider != null) {
        client = debugNetworkImageHttpClientProvider!();
      }
      return true;
    }());
    return client;
  }

The http client is brought by a factory. It could be that this one is returning the same client again.

   factory HttpClient({SecurityContext? context}) {
    HttpOverrides? overrides = HttpOverrides.current;
    if (overrides == null) {
      return _HttpClient(context);
    }
    return overrides.createHttpClient(context);
  }
SimonWeidemann commented 1 year ago

I made a mistake by scoping. The code is working how it should.

Sorry for wasting time.