CI broken by NetworkImage change

[gnprice]

After #1711 which fixed #1710, there's an unrelated CI breakage which happened to also start yesterday.

CI now fails in flutter test, on just 4 tests:

❌ /home/runner/work/zulip-flutter/zulip-flutter/test/widgets/message_list_test.dart: OutboxMessageWithPossibleSender hidden -> waiting (failed)
❌ /home/runner/work/zulip-flutter/zulip-flutter/test/widgets/message_list_test.dart: OutboxMessageWithPossibleSender hidden -> failed, tap to restore message (failed)
❌ /home/runner/work/zulip-flutter/zulip-flutter/test/widgets/message_list_test.dart: OutboxMessageWithPossibleSender hidden -> failed, tapping does nothing if compose box is not offered (failed)
❌ /home/runner/work/zulip-flutter/zulip-flutter/test/widgets/message_list_test.dart: OutboxMessageWithPossibleSender waiting -> waitPeriodExpired, tap to restore message (failed)

The errors look like this:

❌ /home/runner/work/zulip-flutter/zulip-flutter/test/widgets/message_list_test.dart: OutboxMessageWithPossibleSender hidden -> waiting (failed)
  ══╡ EXCEPTION CAUGHT BY IMAGE RESOURCE SERVICE ╞════════════════════════════════════════════════════
  The following NetworkImageLoadException was thrown resolving an image codec:
  HTTP request failed, statusCode: 400, https://chat.example/bar.jpg
  
  When the exception was thrown, this was the stack:
  #0      NetworkImage._loadAsync (package:flutter/src/painting/_network_image_io.dart:130:9)
  <asynchronous suspension>
  #1      MultiFrameImageStreamCompleter._handleCodecReady (package:flutter/src/painting/image_stream.dart:1021:3)
  <asynchronous suspension>
  
  Image provider: NetworkImage("https://chat.example/bar.jpg", scale: 1.0, webHtmlElementStrategy:
    never, headers: {Authorization: Basic
    bWFpbDE0MTlAZXhhbXBsZS5jb206ZFFjRUpXVHEzTGN6b3NEa0puUlR3ZjMxem5pR3ZNck8=, User-Agent:
    ZulipFlutter/1.0.0+1 (Android 13)})
  Image key: NetworkImage("https://chat.example/bar.jpg", scale: 1.0, webHtmlElementStrategy: never,
    headers: {Authorization: Basic
    bWFpbDE0MTlAZXhhbXBsZS5jb206ZFFjRUpXVHEzTGN6b3NEa0puUlR3ZjMxem5pR3ZNck8=, User-Agent:
    ZulipFlutter/1.0.0+1 (Android 13)})
  ════════════════════════════════════════════════════════════════════════════════════════════════════
  Warning: At least one test in this suite creates an HttpClient. When running a test suite that uses
  TestWidgetsFlutterBinding, all HTTP requests will return status code 400, and no network request
  will actually be made. Any test expecting a real network connection and status code will fail.
  To test code that needs an HttpClient, provide your own HttpClient implementation to the code under
  test, so that your test can consistently provide a testable response to the code under test.
  Test failed. See exception logs above.
  The test description was: hidden -> waiting

Diagnosis

As I found at #1711 (comment):

Those tests broke with upstream commit flutter/flutter@97aae2a (flutter/flutter#171916), merged yesterday.

The reason customer_testing didn't prevent that is that the pin there is 834834b, from early last month. The affected tests were introduced in 11f05dc (#1538) and then 7e29108 (#1453), both a week later.

The upstream change is mostly about Flutter web. The one effect outside web is on NetworkImage, making == and hashCode finer-grained and adding more detail to toString. Still TBD is why that causes these tests to fail.

[gnprice]

Aha. The real problem is that there's a state leak in these tests:

The test "MessageWithPossibleSender Updates avatar on RealmUserUpdateEvent" does this:

      Future<void> handleNewAvatarEventAndPump(WidgetTester tester, String avatarUrl) async {
        await store.handleEvent(RealmUserUpdateEvent(id: 1, userId: eg.selfUser.userId, avatarUrl: avatarUrl));
        await tester.pump();
      }
      // …
      await handleNewAvatarEventAndPump(tester, '/bar.jpg');

That mutates the object eg.selfUser. Specifically its avatarUrl field changes from null to /bar.jpg.

As a result, when the later test "OutboxMessageWithPossibleSender hidden -> waiting" or any of its neighbors tries sending a message and causes an outbox-message to appear, it shows the avatar of eg.selfUser… and because the avatar URL is no longer null, that means trying to show a NetworkImage. Those latter tests didn't set debugNetworkImageHttpClientProvider — because they didn't intend to cause any NetworkImage to appear — so that's an error.

The only remaining mystery about the upstream changes is why this was working before. It's not necessary to answer that question, though. The fix for this failure is to make the first test no longer leak state to the other tests.