Skip to content

Conversation

@kiryazovi-redis
Copy link
Contributor

Problem

The test should relax command timeout on MOVING, MIGRATING was flaky in CI, failing with:

Timeout Handling During Notifications should relax command timeout on MOVING, MIGRATING: AssertionError MOVING notification should timeout within relaxed timeout. Duration: 1999.5784149999963, Expected: [2000, 2400) - AssertionError [ERR_ASSERTION]: MOVING notification should timeout within relaxed timeout. Duration: 1999.5784149999963, Expected: [2000, 2400)
at /home/runner/work/cae-client-testing/cae-client-testing/client_src/packages/client/lib/tests/test-scenario/timeout-during-notifications.e2e.ts:2:3572
at Array.forEach ()
at Context. (packages/client/lib/tests/test-scenario/timeout-during-notifications.e2e.ts:2:3336)
at process.processTicksAndRejections (node:internal/process/task_queues:105:5)

Root Cause

The assertion checked that the measured duration was exactly >= RELAXED_COMMAND_TIMEOUT (2000ms). In CI environments, timing precision can vary due to container overhead, CPU scheduling, and other factors, causing the measured duration to occasionally fall slightly below the expected threshold.

Solution

  • Widened the lower bound tolerance from 1.0x to 0.8x of RELAXED_COMMAND_TIMEOUT, allowing durations in the range [1600ms, 2400ms) instead of [2000ms, 2400ms)
  • Added detailed debug information to assertion messages to aid future troubleshooting (shows actual duration and expected range)

Changes

  • timeout-during-notifications.e2e.ts: Relaxed lower bound check and improved assertion messages

@nkaradzhov
Copy link
Collaborator

run scenario tests

@uglide
Copy link
Contributor

uglide commented Dec 3, 2025

Testcase Errors Failures Skipped Total

---- Details for maintainers

@nkaradzhov
Copy link
Collaborator

run scenario tests

@uglide
Copy link
Contributor

uglide commented Dec 5, 2025

Testcase Errors Failures Skipped Total

---- Details for maintainers

@nkaradzhov nkaradzhov self-requested a review December 5, 2025 12:11
@nkaradzhov nkaradzhov merged commit a64134c into redis:master Dec 5, 2025
17 checks passed
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.

3 participants