Skip to content

#289: stop lifetime as normal completion#438

Open
GHX5T-SOL wants to merge 1 commit into
yegor256:masterfrom
GHX5T-SOL:289
Open

#289: stop lifetime as normal completion#438
GHX5T-SOL wants to merge 1 commit into
yegor256:masterfrom
GHX5T-SOL:289

Conversation

@GHX5T-SOL

Copy link
Copy Markdown

Fixes #289.

Stops --lifetime expiration from being reported as a per-judge --timeout error when the lifetime timeout fires while a judge is running. The update now logs lifetime termination, still exports the factbase and churn file, and preserves the existing per-judge --timeout error path.

Validation:

  • env PICKS=yes mise x ruby@3.3 -- bundle exec ruby -Itest -Ilib test/commands/test_update.rb --name '/lifetime|timeout|churn/' -> 6 tests, 17 assertions, 0 failures
  • env PICKS=yes mise x ruby@3.3 -- bundle exec ruby -Itest -Ilib test/commands/test_update.rb -> 28 tests, 64 assertions, 0 failures
  • mise x ruby@3.3 -- bundle exec rubocop lib/judges/commands/update.rb test/commands/test_update.rb -> 2 files inspected, no offenses
  • git diff --check
  • git diff --no-ext-diff | gitleaks stdin --no-banner --redact --exit-code 1 -> no leaks found

Limitations:

  • mise x ruby@3.3 -- bundle exec rake test ran 137 tests and 423 assertions with 0 failures, but hit one local tidy -e .../temp/base.html error in test_print_to_html.
  • Full bundle exec rubocop reports four existing symbol-conversion warnings in unrelated command files.

@yegor256

Copy link
Copy Markdown
Owner

@GHX5T-SOL merge conflict here

@yegor256

Copy link
Copy Markdown
Owner

Please fix the merge conflicts so this pull request can be merged.

1 similar comment
@yegor256

Copy link
Copy Markdown
Owner

Please fix the merge conflicts so this pull request can be merged.

@edmoffo edmoffo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

APPROVE. The fix in lib/judges/commands/update.rb at lines 75-82 drops the conditional re-raise so the lifetime Timeout flows into the ensure block unchanged, and the new branch at lines 258-268 inside one_judge distinguishes a true per-judge --timeout from a lifetime expiration that surfaces inside a judge run, which is exactly what #289 reported. test_terminates_on_lifetime and test_exports_churn_to_file_on_lifetime now assert the completion path instead of expecting a raised error, which matches the new contract. CI is green across rake on ubuntu, macos, and windows, plus actionlint, rubocop, reuse, and the rest. The branch is behind master, so rebasing before merge will produce a smaller integration than the current head-vs-master comparison suggests.

@0crat

0crat commented Jun 19, 2026

Copy link
Copy Markdown

@edmoffo Thanks for the review! You've earned +10 points for this: +18 as a basis; -8 for absolutely no comments posted. Your running score is +1913; don't forget to check your Zerocracy account too).

@yegor256

yegor256 commented Jul 7, 2026

Copy link
Copy Markdown
Owner

@GHX5T-SOL merge confict here

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.

Termination on timeout is too aggressive

4 participants