Skip to content

Conversation

@dumbbell
Copy link
Collaborator

@dumbbell dumbbell commented Feb 24, 2025

  • amqp_client_SUITE: Use a dedicated CI job for this testsuite
  • amqp10_client: Handle close message in the open_sent state
  • amqp10_client: Fix crash in close_sent
  • amqp_client_SUITE: Retry connection in two testcases
  • amqp_client_SUITE: Ensure idle_time_out_on_server restores heartbeat value
  • amqp_client_SUITE: Use a dedicated AMQP-0-9-1 connection per testcase
  • amqp_client_SUITE: Close all connections in end_per_testcase/2

@dumbbell dumbbell self-assigned this Feb 24, 2025
@dumbbell dumbbell changed the title amqp_client_SUITE: Ensure testcases clean up connection and session amqp_client_SUITE: Ensure testcases clean up connections and sessions Feb 24, 2025
@dumbbell dumbbell changed the title amqp_client_SUITE: Ensure testcases clean up connections and sessions amqp_client_SUITE: Fix frequent test failures Feb 24, 2025
@mergify mergify bot added the make label Feb 25, 2025
@dumbbell dumbbell force-pushed the fix-test-flakes-in-amqp_client_SUITE branch 6 times, most recently from f38acc2 to 804ac4e Compare February 27, 2025 08:35
@ansd
Copy link
Member

ansd commented Feb 27, 2025

I wonder whether instead of using try ... after in each test case, it would be better to force close any connections in end_per_testcase. This way the test cases are more readable. WDYT?

@dumbbell dumbbell force-pushed the fix-test-flakes-in-amqp_client_SUITE branch 2 times, most recently from 4188c23 to b6cb91a Compare March 3, 2025 08:23
@dumbbell
Copy link
Collaborator Author

dumbbell commented Mar 3, 2025

I wonder whether instead of using try ... after in each test case, it would be better to force close any connections in end_per_testcase. This way the test cases are more readable. WDYT?

We discussed this with David outside of GitHub and I agreed this is a good solution.

@dumbbell dumbbell force-pushed the fix-test-flakes-in-amqp_client_SUITE branch from b6cb91a to 45c23e0 Compare March 3, 2025 09:50
@dumbbell dumbbell force-pushed the fix-test-flakes-in-amqp_client_SUITE branch 2 times, most recently from 10d6b12 to 0c79228 Compare March 7, 2025 13:06
dumbbell and others added 7 commits March 7, 2025 14:48
[Why]
This testsuite is very unstable and it is difficult to debug while it is
part of a `parallel-ct` group. It also forced us to re-run the entire
`parallel-ct` group just to retry that one testsuite.
[Why]
Without this, the connection process crashes. We see this happenning in
CI frequently.
Fix crash in close_sent since the client might receive the open frame if
it previously sent the close frame in state open_sent.

We explicitly ignore the open frame. The alternative is to add another
gen_statem state CLOSE_PIPE which  might be an overkill however.

This commit also fixes a wrong comment: No sessions have begun if the
app requests the connection to be closed in state open_sent.
The testcases are `leader_transfer_credit` and
`dead_letter_into_stream`.
…t value

[Why]
If the testcase fails, it was leaving the low heartbeat value in place,
leading to many subsequent tests to fail.
... instead of a global one. Otherwise, one connection failure, even if
expected by a testcase, will affect all subsequent testcases negatively.
[Why]
Many tests do not clean up their connections if they encounter a
failure. This affects subsequent testcases negatively.
@dumbbell dumbbell force-pushed the fix-test-flakes-in-amqp_client_SUITE branch from 0c79228 to 4d12efa Compare March 7, 2025 13:49
@dumbbell dumbbell marked this pull request as ready for review March 7, 2025 14:45
@ansd ansd self-requested a review March 7, 2025 14:49
@dumbbell dumbbell merged commit f661c1e into main Mar 7, 2025
273 checks passed
@dumbbell dumbbell deleted the fix-test-flakes-in-amqp_client_SUITE branch March 7, 2025 16:52
@michaelklishin
Copy link
Collaborator

@Mergifyio backport v4.1.x v4.0.x

@mergify
Copy link

mergify bot commented Mar 8, 2025

backport v4.1.x v4.0.x

✅ Backports have been created

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants