Skip to content

Increase timeout for test_lock_contention on RISC-V #18430

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

Gui-Yue
Copy link

@Gui-Yue Gui-Yue commented May 12, 2025

This PR addresses a test failure for tests.handlers.test_worker_lock.WorkerLockTestCase.test_lock_contention which consistently times out on the RISC-V (specifically riscv64) architecture.

The test simulates high lock contention and has a default timeout of 5 seconds, which seems sufficient for architectures like x86_64 but proves too short for current RISC-V hardware/environment performance characteristics, leading to spurious tests.utils.TestTimeout failures.

This fix introduces architecture detection using platform.machine(). If a RISC-V architecture is detected:

  • The timeout for this specific test is increased (e.g., to 15 seconds ).

The original, stricter timeout (5 seconds) and lock count (500) are maintained for all other architectures to avoid masking potential performance regressions elsewhere.

This change has been tested locally on RISC-V, where the test now passes reliably, and on x86_64, where it continues to pass with the original constraints.


Pull Request Checklist

  • Pull request is based on the develop branch (Assuming you based it correctly)
  • Pull request includes a changelog file. (See below)
  • Code style is correct (run the linters) (Please run linters locally)

@Gui-Yue Gui-Yue requested a review from a team as a code owner May 12, 2025 14:13
@CLAassistant
Copy link

CLAassistant commented May 12, 2025

CLA assistant check
All committers have signed the CLA.

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.

2 participants