build icon indicating copy to clipboard operation
build copied to clipboard

Use WebSocket fakes

Open natebosch opened this issue 1 year ago • 9 comments

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 avatar Jun 04 '24 20:06 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.

natebosch avatar Jun 04 '24 21:06 natebosch

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.

brianquinlan avatar Jun 04 '24 21:06 brianquinlan

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'

natebosch avatar Jun 05 '24 00:06 natebosch

Let me patch this in and take a look.

brianquinlan avatar Jun 06 '24 21:06 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 avatar Jun 06 '24 22:06 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.

brianquinlan avatar Jun 06 '24 23:06 brianquinlan

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 avatar Jun 07 '24 20:06 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.

natebosch avatar Jun 11 '24 00:06 natebosch

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

No concerns

jakemac53 avatar Jun 11 '24 15:06 jakemac53

Very old draft, please re-open if you want to do something with it :)

davidmorgan avatar Jul 07 '25 09:07 davidmorgan