Skip to content

bug(#767): add explicit timeouts to slow tests, puzzle for lint performance optimization#768

Merged
yegor256 merged 1 commit intoobjectionary:masterfrom
h1alexbel:767
Sep 30, 2025
Merged

bug(#767): add explicit timeouts to slow tests, puzzle for lint performance optimization#768
yegor256 merged 1 commit intoobjectionary:masterfrom
h1alexbel:767

Conversation

@h1alexbel
Copy link
Member

@h1alexbel h1alexbel commented Sep 30, 2025

closes #767

Summary by CodeRabbit

  • Tests
    • Increased timeouts for several long-running tests to improve stability and reduce flakiness.
    • Standardized timeout units and added per-test timeouts where appropriate.
  • Documentation
    • Added notes in test documentation regarding lint timeouts and performance considerations.

No user-facing functionality changes.

@coderabbitai
Copy link

coderabbitai bot commented Sep 30, 2025

Walkthrough

Per-test timeout annotations were added or adjusted in three test classes to mitigate previous timeout failures. Imports for java.util.concurrent.TimeUnit were introduced where needed. One class gained a Javadoc TODO note about performance and timeouts. No test logic or control flow was modified.

Changes

Cohort / File(s) Summary of Changes
Per-test timeout adjustments (Groovy)
src/test/groovy/org/eolang/lints/HomeNamesTest.groovy
Added TimeUnit import; added @timeout(unit = MINUTES, value = 3L) to two tests alongside existing class-level timeout; no logic changes.
Per-test timeout adjustments (Java A)
src/test/java/org/eolang/lints/LtByXslTest.java
Imported TimeUnit; changed @timeout from 180L seconds to 5 minutes using unit/value form.
Per-test timeout adjustments (Java B) + note
src/test/java/org/eolang/lints/SourceTest.java
Imported TimeUnit; set 60-second timeout for returnsEmptyListOfDefects; increased deep test timeout to 2 minutes; added class-level Javadoc TODO about timeouts/performance; standardized explicit unit usage.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I tapped my paw and watched the clock,
Tests ran long—tick-tock, tick-tock!
We stretched the bounds, a gentle nudge,
Minutes granted, no more sludge.
Now builds hop by with timely cheer—
A rabbit’s fix: more time, less fear! 🐇⏱️

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning A Javadoc todo note block was added to SourceTest.java concerning future lint performance improvements, which is unrelated to the immediate objective of fixing test timeouts and therefore constitutes an out-of-scope change. Please remove the Javadoc todo note or move it to a separate documentation or performance-optimization pull request so that this PR remains focused solely on extending test timeouts.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title correctly highlights the main change of adding explicit timeouts to slow tests as per issue #767, but it also includes an extra clause about a “puzzle for lint performance optimization” that is not directly addressed in this changeset and may distract from the core intent.
Linked Issues Check ✅ Passed The code changes introduce or update timeouts for every test listed in issue #767: HomeNamesTest methods now timeout after three minutes, LtByXslTest.validatesEoPacksForErrors after five minutes, SourceTest.returnsEmptyListOfDefects after sixty seconds, and both SourceTest.lintsInMultipleThreads instances after two minutes, thereby directly addressing each reported timeout failure without omitting any required adjustments.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e751958 and f98fb28.

📒 Files selected for processing (3)
  • src/test/groovy/org/eolang/lints/HomeNamesTest.groovy (3 hunks)
  • src/test/java/org/eolang/lints/LtByXslTest.java (2 hunks)
  • src/test/java/org/eolang/lints/SourceTest.java (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/test/java/org/eolang/lints/SourceTest.java (1)
src/test/java/org/eolang/lints/LtByXslTest.java (1)
  • SuppressWarnings (57-414)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: reserved
  • GitHub Check: deep
  • GitHub Check: ort
  • GitHub Check: mvn (windows-2022, 21)
  • GitHub Check: mvn (ubuntu-24.04, 11)
  • GitHub Check: mvn (windows-2022, 11)
  • GitHub Check: mvn (macos-15, 11)
  • GitHub Check: mvn (macos-15, 21)
  • GitHub Check: mvn (ubuntu-24.04, 21)
🔇 Additional comments (9)
src/test/java/org/eolang/lints/LtByXslTest.java (2)

21-21: LGTM! Import necessary for explicit TimeUnit usage.

The import supports the more explicit timeout annotation below.


363-363: Timeout increased from 3 to 5 minutes to prevent failures.

The timeout was extended from 180 seconds (3 minutes) to 5 minutes, addressing the timeout failure reported in issue #767. The explicit TimeUnit specification improves code clarity.

src/test/groovy/org/eolang/lints/HomeNamesTest.groovy (3)

10-10: LGTM! Import necessary for explicit TimeUnit usage.

The import supports the TimeUnit.MINUTES specification in the timeout annotations below.


28-28: Per-test timeout added to address failure.

The 3-minute timeout addresses the timeout failure reported in issue #767 for this test (previously timed out at 90 seconds). Note that per-test timeouts override the class-level 90-second timeout at line 23.


40-40: Per-test timeout added for consistency.

The 3-minute timeout matches the sibling test and provides adequate headroom for this test method, overriding the class-level 90-second timeout.

src/test/java/org/eolang/lints/SourceTest.java (4)

31-31: LGTM! Import necessary for explicit TimeUnit usage.

The import supports the TimeUnit specifications in the timeout annotations below.


65-68: Good practice: TODO documents technical debt.

The puzzle correctly acknowledges that these timeouts are symptomatic of slow lints and tracks the need for performance optimization. The 60-minute estimate and clear action items make this actionable.


75-75: Timeout added to prevent failure.

The 60-second timeout addresses the failure reported in issue #767 (test timed out at 45 seconds). Using explicit TimeUnit.SECONDS improves clarity.


153-153: Timeout doubled to 2 minutes for repeated test.

The timeout was increased from 60 seconds to 2 minutes, addressing the failures reported in issue #767 for lintsInMultipleThreads()[1] and [2]. Since this is a @RepeatedTest(2) at line 154, the generous timeout accommodates both repetitions.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🧪 Early access (Sonnet 4.5): enabled

We are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience.

Note:

  • Public repositories are always opted into early access features.
  • You can enable or disable early access features from the CodeRabbit UI or by updating the CodeRabbit configuration file.

Comment @coderabbitai help to get the list of available commands and usage tips.

@h1alexbel
Copy link
Member Author

@yegor256 please check

@yegor256 yegor256 merged commit d4c6c55 into objectionary:master Sep 30, 2025
25 checks passed
@yegor256
Copy link
Member

@h1alexbel thanks!

@h1alexbel h1alexbel deleted the 767 branch September 30, 2025 15:42
@0crat
Copy link

0crat commented Oct 1, 2025

@yegor256 Thanks for the review! You've earned +4 points for this contribution: +12 as the base reward, -8 for no comments posted during the review, and -4 for having fewer than 32 hits-of-code (14 total), with a +4 adjustment to meet the minimum threshold. Your running balance is +17; don't forget to check your Zerocracy balance too.

@0crat
Copy link

0crat commented Nov 6, 2025

@h1alexbel Hey there! Nice work on your contribution – you've snagged +12 points this time: +16 as the base reward, but -4 got deducted since you only hit 14 hits-of-code (our policy dings contributions under 40 HoC). Next time, try to push past that 40 HoC threshold to avoid the penalty and maximize your points! 🚀 Keep them coming – your running score is looking solid at +76, and don't forget to peek at your Zerocracy account for the latest updates.

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.

Rultor release fails due to multiple test timeouts

3 participants