Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix all hanging tests, reduce flakyness of other tests #2186

Open
wants to merge 71 commits into
base: main
Choose a base branch
from

Conversation

ptesavol
Copy link
Collaborator

@ptesavol ptesavol commented Dec 8, 2023

Fix all hanging tests. Includes a protobuf change.

  • Bugfix: change gracefulDiconnect into notification (it was previously partly treated as a notification)

  • Bugfix: do not open new connections when ConnectionManager is in Stopping state

  • Make timeouts unique by single milliseconds to make it easier to recognize in debug tools which timeout is which

  • Force SimulatorConnection to send 'disconnect' event in close() the same way as websocket connections

  • Add constructor option 'stopGivenTransport' DhtNode to indicate that the given transport should not be stopped when the DhtNode is stopped.

  • Use the 'stopGivenTransport' in trackerless tests (the transports were not previously stopped at all)

  • Call stop() on simulator in tests if it was started

  • remove --force-exit from test scripts

  • remove memory leaks from client by adding handling of destroy signal to many places and by removing closures

  • fix mqtt bridge test to properly wait for arriving messages

  • fix websocket connection establishment in broker websocket plugin test (previously 'error' waitForEvent would time out and cause unhandled promise exception on slower machines)

  • await for sub1.unsubscribe() in memory leak test to get the subscription to non-random state before testing for memory leaks

…udes a protobuf change.

* Bugfix: change gracefulDiconnect into notification (it was previously partly treated as a notification)
* Bugfix: do not open new connections when ConnectionManager is in Stopping state

* Make timeouts unique by single milliseconds to make it easier to recognize in debug tools which timeout is which
* Force SimulatorConnection to send 'disconnect' event in close() the same way as websocket connections
* Add constructor option 'stopGivenTransport' DhtNode to indicate that the given transport should not be stopped when the DhtNode is stopped.
* Use the 'stopGivenTransport' in trackerless tests (the transports were not previously stopped at all)
* Call stop() on simulator in tests if it was started
@github-actions github-actions bot added network Related to Network Package dht Related to DHT package labels Dec 8, 2023
@@ -68,6 +69,10 @@ describe('inspect', () => {
Simulator.useFakeTimers(false)
})

afterAll(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move to afterEach

@@ -32,6 +32,9 @@ export class SimulatorConnector {
}

public connect(targetPeerDescriptor: PeerDescriptor): ManagedConnection {
if (this.stopped) {
console.error('connect() called on stopped SimulatorConnector')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove console.error

@@ -30,7 +30,8 @@ describe('inspect', () => {
layer0: {
entryPoints: [publisherDescriptor],
peerDescriptor,
transport
transport,
stopGivenTransport: true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could use the same pattern as in dht test utils:

const node = new class extends DhtNode {
async stop(): Promise<void> {

Or we could add e.g. stop event which the creator of the instance could listen.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about passing a transport factory instead the transport itself?

…m:streamr-dev/network-monorepo into fix-all-dht-and-trackerless-hanging-tests
@github-actions github-actions bot added the client Related to Client Package label Dec 8, 2023
@github-actions github-actions bot added the ci Related to CI configuration label Dec 9, 2023
@github-actions github-actions bot added the test-utils Related to Test Utils Package label Dec 9, 2023
@github-actions github-actions bot added the broker Related to Broker Package label Dec 9, 2023
Base automatically changed from streamr-1.0 to main December 27, 2023 08:31
teogeb added a commit that referenced this pull request Jan 3, 2024
Change the response of `gracefulDiconnect` to a notification (it was previously partly treated as a notification).

This is a cherry-pick from #2186, i.e. the original author is @ptesavol.
teogeb added a commit that referenced this pull request Jan 3, 2024
…2261)

Do not send messages when `ConnectionManager` is in `STOPPING` state.

This is a cherry-pick from #2186, i.e. the original author is @ptesavol.
@harbu harbu changed the title Fix all hanging tests, reduce flakyness of other tests fix: Fix all hanging tests, reduce flakyness of other tests Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
broker Related to Broker Package ci Related to CI configuration cli-tools Related to CLI Tools Package client Related to Client Package dht Related to DHT package network Related to Network Package test-utils Related to Test Utils Package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants