oven-sh / bun

Incredibly fast JavaScript runtime, bundler, test runner, and package manager – all in one
https://bun.sh
Other
73.96k stars 2.75k forks source link

Add test coverage for `EventEmitter` #1942

Closed ThatOneBro closed 1 year ago

ThatOneBro commented 1 year ago

Currently EventEmitter can be pretty finicky at the edges where it touches other APIs, especially when interacting with bun:test and a few other places like node:stream.

It would probably be good to:

  1. Add test coverage for EventEmitter + bun:test to make sure all edge and corner cases are dealt with appropriately. This is includes where done is called inside of event listeners, for both passing and failing expect statements. There are probably other cases which should be tested to, but this is the one I've just encountered that was causing the runner to hang.
  2. Copy Node's tests for EventEmitter over, so we can make sure our EventEmitter conforms exactly to Node's spec, since Node's APIs really like to use EventEmitter a lot.

This should help improve the stability of a large segment of Bun's Node polyfills and places where Bun's APIs interact with them.

Electroid commented 1 year ago

https://github.com/oven-sh/bun/blob/85bde43c2f1adfda348d20282c85f31152b4cd2a/test/js/node/events/event-emitter.test.ts#L1-L100