Skip to content

Conversation

@alarso16
Copy link
Contributor

@alarso16 alarso16 commented Oct 23, 2025

Why this should be merged

Closes #1356 as pre-req for monorepo. These tests were hazy in scope anyway and didn't accurately test what it intended to.

How this works

It removes one of the tests, and combines two of the tests utilizing the AppGossip function instead.

How this was tested

UT

Need to be documented?

No

Need to update RELEASES.md?

No

@alarso16 alarso16 self-assigned this Oct 23, 2025
@alarso16 alarso16 added bug Something isn't working ci testing Anything testing-related requires-subnet-evm-port labels Oct 23, 2025
@alarso16 alarso16 marked this pull request as ready for review October 23, 2025 20:49
@alarso16 alarso16 requested a review from a team as a code owner October 23, 2025 20:49
Copy link
Member

@JonathanOppenheimer JonathanOppenheimer left a comment

Choose a reason for hiding this comment

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

I'm also not positive this will fix the flakes, but the locking makes sense to me.

@alarso16 alarso16 force-pushed the alarso16/mempool-flakes branch from bfd82ee to 34355e9 Compare October 27, 2025 16:49
Copy link
Contributor

@StephenButtolph StephenButtolph left a comment

Choose a reason for hiding this comment

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

Added comments for what these tests previously actually did.

We should decide if that behavior is worth testing (and if it is we should document it correctly).

@alarso16 alarso16 force-pushed the alarso16/mempool-flakes branch from 4bdb916 to 2a6d166 Compare October 27, 2025 20:57
@StephenButtolph StephenButtolph added this pull request to the merge queue Oct 28, 2025
@StephenButtolph StephenButtolph removed this pull request from the merge queue due to a manual request Oct 28, 2025
Signed-off-by: Stephen Buttolph <stephen@avalabs.org>
@StephenButtolph StephenButtolph added this pull request to the merge queue Oct 28, 2025
Merged via the queue into master with commit 0343a1b Oct 28, 2025
9 checks passed
@StephenButtolph StephenButtolph deleted the alarso16/mempool-flakes branch October 28, 2025 17:24
// Tests that conflicting txs are handled, based on whether the original tx is
// valid or not.
func TestAtomicTxPushGossipInboundConflicting(t *testing.T) {
require := require.New(t)
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need this. The only thing it achieves is implicitly passing testing.T to the concrete require assertion call, but it adds unnecessary noise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ci requires-subnet-evm-port testing Anything testing-related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix mempool test flakes

6 participants