Use WebSocket fakes

[natebosch]

Refactor serve_handler_test to use the fakes utility from the
web_socket package instead of reimplement it locally.

Currently this changes the behavior of the tests and they fail.

[natebosch]

cc @brianquinlan - are behavior differences expected from this type of refactoring? I would have hoped that the different fake implementations behave the same, and that wrapping a WebSocket in an adapter should work the same as a WebSocketChannel.

Opening this as the PR to have a place to see and discuss the differences.

[brianquinlan]

I'm surprised that the tests fail. Are you seeing the failures locally because the github CI seems to get hung-up at the analysis phase.

[natebosch]

Ah yeah, I had hoped to see the error on CI too, I'll fix up analysis and copy the logs in the meantime.

These stream matcher tests are unfortunately not very nice to debug so the log output isn't as useful as making changes in the test.

local failure

00:01 +17 -1: build updates WebSocket handler emmits a message to all listners [E]                 
  Expected: should do the following in order:
            • emit an event that '{}'
            • be done
    Actual: <Instance of '_ControllerStream<Object?>'>
     Which: emitted x Stream closed.
              which didn't emit an event that '{}'
  
  package:test_api  OutstandingWork.complete
  

To run this test again: dart test test/server/serve_handler_test.dart -p vm --plain-name 'build upda
tes WebSocket handler emmits a message to all listners'
00:01 +17 -2: build updates WebSocket handler deletes listners on disconect [E]                    
  Expected: should do the following in order:
            • emit an event that '{}'
            • be done
    Actual: <Instance of '_ControllerStream<Object?>'>
     Which: emitted x Stream closed.
              which didn't emit an event that '{}'
  
  package:test_api  OutstandingWork.complete
  
  Expected: should do the following in order:
            • emit an event that '{}'
            • emit an event that '{}'
            • be done
    Actual: <Instance of '_ControllerStream<Object?>'>
     Which: emitted • {}
                    x Stream closed.
              which didn't emit an event that '{}'
  
  package:test_api  OutstandingWork.complete
  

To run this test again: dart test test/server/serve_handler_test.dart -p vm --plain-name 'build upda
tes WebSocket handler deletes listners on disconect'
00:01 +18 -3: build updates WebSocket handler closes listners [E]                                  
  Concurrent modification during iteration: Instance(length:0) of '_GrowableList'.
  dart:async                                          Future.wait
  package:build_runner/src/server/server.dart 253:19  BuildUpdatesWebSocketHandler.close
  test/server/serve_handler_test.dart 355:23          main.<fn>.<fn>.<fn>
  

To run this test again: dart test test/server/serve_handler_test.dart -p vm --plain-name 'build upda
tes WebSocket handler closes listners'
00:01 +18 -4: build updates WebSocket handler emmits build results digests [E]                     
  Expected: should do the following in order:
            • emit an event that {'index.html': 'c6ff69e8720d3a6d523a16d5760f6dbe'}
            • emit an event that {
                'index.html': 'c6ff69e8720d3a6d523a16d5760f6dbe',
                'packages/a/some.dart.js': '1904ce447fbfec58946857910df4936d'
              }
            • be done
    Actual: <Instance of '_MapStream<Object?, dynamic>'>
     Which: emitted • {index.html: c6ff69e8720d3a6d523a16d5760f6dbe}
                    x Stream closed.
              which didn't emit an event that {
                      'index.html': 'c6ff69e8720d3a6d523a16d5760f6dbe',
                      'packages/a/some.dart.js': '1904ce447fbfec58946857910df4936d'
                    }
  
  package:test_api  OutstandingWork.complete
  

To run this test again: dart test test/server/serve_handler_test.dart -p vm --plain-name 'build upda
tes WebSocket handler emmits build results digests'
00:01 +18 -5: build updates WebSocket handler works for different root dirs [E]                    
  Expected: should do the following in order:
            • emit an event that {
                'index.html': '494467021b8a4a1ad5f34c3ba081b649',
                'packages/a/some.dart.js': '53c8b3a21466133312fc12ef15c44619'
              }
            • be done
    Actual: <Instance of '_MapStream<Object?, dynamic>'>
     Which: emitted x Stream closed.
              which didn't emit an event that {
                      'index.html': '494467021b8a4a1ad5f34c3ba081b649',
                      'packages/a/some.dart.js': '53c8b3a21466133312fc12ef15c44619'
                    }
  
  package:test_api  OutstandingWork.complete
  

To run this test again: dart test test/server/serve_handler_test.dart -p vm --plain-name 'build updates WebSocket handler works for different root dirs'

[brianquinlan]

Let me patch this in and take a look.

[brianquinlan]

The data seems to be flowing correctly but something is probably getting closed or not listed too early. The tests pass with this change

      test('emmits a message to all listners', () async {
        expect(clientChannel1.stream, emitsInOrder(['{}', emitsDone]));
        expect(clientChannel2.stream, emitsInOrder(['{}', emitsDone]));
        await createMockConnection(serverChannel1, 'web');
        await createMockConnection(serverChannel2, 'web');
        await handler.emitUpdateMessage(BuildResult(BuildStatus.success, []));
+       await Future.delayed(const Duration(seconds: 5));
        await clientChannel1.sink.close();
        await clientChannel2.sink.close();
      });

[brianquinlan]

The data gets received by the fake clientChannel1/clientChannel2 and this line in adapter_web_socket_channel.dart gets executed:

          case TextDataReceived(text: final text):
            _controller.local.sink.add(text);

But the data does not seem to appear in the _controller.foreign.stream. StreamChannelController is a bit hard to follow.

[natebosch]

I'll play around with this some more next week. If the failure is only exposing some way that the test was too sensitive about async interleaving then I'm not too concerned about the difference impacting production use.

[natebosch]

I added some await pumpEventQueue(); and it fixes most of the tests. I don't think this particular extra async event is anything that would impact production code (@jakemac53 let me know if you have any concerns about needing these test changes).

There is one remaining failure which I'll look at more tomorrow.

[jakemac53]

@jakemac53 let me know if you have any concerns about needing these test changes).

No concerns