Skip to content

Commit

Permalink
fix: fix flaky port number assignments in cucumber (#4305)
Browse files Browse the repository at this point in the history
Description
---
Fixed flaky port cucumber number assignments whereby under certain conditions the same port number were re-used resulting in test failures. This fix serializes ports returned by `const getFreePort = function ()` with a much higher probability for the ports to be unique. In the event that base node creation still fails (e.g. due to port conflicts), the error is catched and the process is retried.

A recent example of this in [`run-integration-tests`](https://app.circleci.com/pipelines/github/tari-project/tari/16711/workflows/8aedce83-a2fb-4bbc-b0f4-ff85ff76a627/jobs/42246/parallel-runs/0/steps/0-111) for test `Testing scenario: "When a new node joins the network, it receives all peers"` is shown below where port `45657` was re-used resulting in test failure:

```
Port: 45343
GRPC: 45657
Starting node Basenode45343-seed-SeedNode6...

...

Port: 45657
GRPC: 39549
Starting node Basenode45657-seed-SeedNode0...

...

stderr: 15:26 WARN  PeerListener was unable to start because 'Failed to listen on /ip4/127.0.0.1/tcp/45657: 
  Address already in use (os error 98)'
15:26 ERROR Failed to start listener(s). Failed to listen on /ip4/127.0.0.1/tcp/45657: Address already in use 
  (os error 98). Connection manager is quitting.
```

Motivation and Context
---
Some cucumber tests were flaky due to duplicate port assignments.

How Has This Been Tested?
---
Cucumber tests
  • Loading branch information
hansieodendaal committed Jul 13, 2022
1 parent e1704f4 commit e8d4a00
Show file tree
Hide file tree
Showing 2 changed files with 113 additions and 25 deletions.
95 changes: 81 additions & 14 deletions integration_tests/features/support/node_steps.js
Expand Up @@ -38,12 +38,34 @@ Given(/I have a seed node (.*)/, { timeout: 20 * 1000 }, async function (name) {
return await this.createSeedNode(name);
});

Given("I have {int} seed nodes", { timeout: 20 * 1000 }, async function (n) {
const promises = [];
Given("I have {int} seed nodes", { timeout: 60 * 1000 }, async function (n) {
for (let i = 0; i < n; i++) {
promises.push(this.createSeedNode(`SeedNode${i}`));
const seedName = `SeedNode${i}`;
let initialized = false;
let count = 0;
while (!initialized) {
try {
await this.createSeedNode(seedName);
initialized = true;
} catch {
if (this.clients[seedName] !== undefined) {
delete this.clients[seedName];
}
if (this.seeds[seedName] !== undefined) {
await this.seeds[seedName].stop();
delete this.seeds[seedName];
}
count += 1;
if (count >= 10) {
console.log(
seedName,
"could not be initialized, suspecting port conflicts"
);
expect(initialized).to.equal(true);
}
}
}
}
await Promise.all(promises);
});

Given(
Expand Down Expand Up @@ -131,9 +153,33 @@ Given(
}
);

Given("I have {int} base nodes", { timeout: 20 * 1000 }, async function (n) {
Given("I have {int} base nodes", { timeout: 60 * 1000 }, async function (n) {
for (let i = 0; i < n; i++) {
await this.createAndAddNode(`basenode${i}`);
const nodeName = `BaseNode${i}`;
let initialized = false;
let count = 0;
while (!initialized) {
try {
await this.createAndAddNode(nodeName);
initialized = true;
} catch {
if (this.clients[nodeName] !== undefined) {
delete this.clients[nodeName];
}
if (this.nodes[nodeName] !== undefined) {
await this.nodes[nodeName].stop();
delete this.nodes[nodeName];
}
count += 1;
if (count >= 10) {
console.log(
nodeName,
"could not be initialized, suspecting port conflicts"
);
expect(initialized).to.equal(true);
}
}
}
}
});

Expand Down Expand Up @@ -197,17 +243,38 @@ Given(

Given(
"I have {int} base nodes connected to all seed nodes",
{ timeout: 20 * 1000 },
{ timeout: 60 * 1000 },
async function (n) {
const promises = [];
for (let i = 0; i < n; i++) {
const miner = this.createNode(`BaseNode${i}`);
miner.setPeerSeeds([this.seedAddresses()]);
promises.push(
miner.startNew().then(() => this.addNode(`BaseNode${i}`, miner))
);
const nodeName = `BaseNode${i}`;
let initialized = false;
let count = 0;
while (!initialized) {
try {
const miner = this.createNode(nodeName);
miner.setPeerSeeds([this.seedAddresses()]);
await miner.startNew();
await this.addNode(nodeName, miner);
initialized = true;
} catch {
if (this.clients[nodeName] !== undefined) {
delete this.clients[nodeName];
}
if (this.nodes[nodeName] !== undefined) {
await this.nodes[nodeName].stop();
delete this.nodes[nodeName];
}
count += 1;
if (count >= 10) {
console.log(
nodeName,
"could not be initialized, suspecting port conflicts"
);
expect(initialized).to.equal(true);
}
}
}
}
await Promise.all(promises);
}
);

Expand Down
43 changes: 32 additions & 11 deletions integration_tests/features/support/steps.js
Expand Up @@ -618,20 +618,41 @@ Then(

When(
"I have {int} base nodes with pruning horizon {int} force syncing on node {word}",
{ timeout: 20 * 1000 },
{ timeout: 60 * 1000 },
async function (nodes_count, horizon, force_sync_to) {
const promises = [];
const force_sync_address = this.getNode(force_sync_to).peerAddress();
for (let i = 0; i < nodes_count; i++) {
const base_node = this.createNode(`BaseNode${i}`, {
pruningHorizon: horizon,
});
base_node.setPeerSeeds([force_sync_address]);
base_node.setForceSyncPeers([force_sync_address]);
promises.push(
base_node.startNew().then(() => this.addNode(`BaseNode${i}`, base_node))
);
const nodeName = `BaseNode${i}`;
let initialized = false;
let count = 0;
while (!initialized) {
try {
const base_node = this.createNode(nodeName, {
pruningHorizon: horizon,
});
base_node.setPeerSeeds([force_sync_address]);
base_node.setForceSyncPeers([force_sync_address]);
await base_node.startNew();
await this.addNode(nodeName, base_node);
initialized = true;
} catch {
if (this.clients[nodeName] !== undefined) {
delete this.clients[nodeName];
}
if (this.nodes[nodeName] !== undefined) {
await this.nodes[nodeName].stop();
delete this.nodes[nodeName];
}
count += 1;
if (count >= 10) {
console.log(
nodeName,
"could not be initialized, suspecting port conflicts"
);
expect(initialized).to.equal(true);
}
}
}
}
await Promise.all(promises);
}
);

0 comments on commit e8d4a00

Please sign in to comment.