Skip to content

fix some common test flakes#63

Merged
iliana merged 3 commits intorelease-22.1-oxidefrom
iliana/flake
Mar 13, 2026
Merged

fix some common test flakes#63
iliana merged 3 commits intorelease-22.1-oxidefrom
iliana/flake

Conversation

@iliana
Copy link
Copy Markdown
Collaborator

@iliana iliana commented Mar 13, 2026

No description provided.

@iliana iliana linked an issue Mar 13, 2026 that may be closed by this pull request
require.Eventually(t, func() bool {
return nd.ConnHealthTryDial(staticNodeID, rpc.DefaultClass) == nil
}, time.Second, 10*time.Millisecond)
}, 5*time.Second, 10*time.Millisecond)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Closing the connection pops the breaker. The lines just above this allow 5 seconds for a popped breaker to recover. So why not here?

// Set up an empty 9-node cluster with simulated latencies.
demoCtx.SimulateLatency = true
demoCtx.NumNodes = 9
demoCtx.Localities = defaultLocalities
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not really sure why this fixes things and it kind of pisses me off. Best I can tell this ends up being nil if it's not set, so maybe this test never worked properly and any simulated latencies were an accident?

Comment on lines -1367 to -1373
if tc.expectNonTrivialSchemaChangeTime && postCommitJobsInterval.AsFloat64() < 0.1 {
t.Fatalf(
"expected schema changes to take longer than 0.1 seconds, took: %v",
postCommitJobsInterval.AsFloat64(),
)
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Failing the test because it took less than 100ms to insert and alter 10,000 rows is ridiculous. I think the expectNonTrivialSchemaChangeTime is really meant to make sure that the other queries don't take too long, not to make sure that this query takes long enough...

@iliana iliana merged commit 645c456 into release-22.1-oxide Mar 13, 2026
14 checks passed
@iliana iliana deleted the iliana/flake branch March 13, 2026 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test flake in pkg/cli/democluster: TestTransientClusterSimulateLatencies

1 participant