Skip to content

test: assert the import-lock precondition instead of assuming it - #429

Merged
fabiodalez-dev merged 1 commit into
mainfrom
test/import-lock-precondition
Sep 14, 2026
Merged

fabiodalez-dev merged 1 commit into
mainfrom
test/import-lock-precondition

Conversation

@fabiodalez-dev

@fabiodalez-dev fabiodalez-dev commented Sep 14, 2026 •

Copy link
Copy Markdown
Owner

Test-only fix for the flaky import-lock contention check that blocked the 0.7.84 release preflight: the precondition (the blocking connection actually holding the lock) is now asserted instead of assumed, and both messages carry the observed evidence.

Summary by CodeRabbit

  • Test
    • Migliorata la verifica dell’importazione CSV in condizioni di contesa, assicurando che il blocco previsto sia effettivamente acquisito prima del test.
    • Aggiunti messaggi diagnostici più dettagliati in caso di errore e gestione esplicita del rilascio del blocco al termine della verifica.

The contention check blocked a release: it held the lock on a second
connection, then required commit() to refuse — but never verified that the
second connection had actually acquired the lock. When it had not, commit()
legitimately succeeded and the check failed for a reason unrelated to what it
tests, intermittently and without saying anything useful.

The precondition is now asserted (with a couple of retries), and both messages
carry what they observed: the outcome, the measured wait, the lock name and
who holds it. A failure now either names a real defect or explains itself.
@coderabbitai

coderabbitai Bot commented Sep 14, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 61c4cff4-2e94-4836-ac37-563fa833110a

📥 Commits

Reviewing files that changed from the base of the PR and between 4c0ec64 and 0c2fcba.

📒 Files selected for processing (1)
  • tests/emeroteca-412.unit.php

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Il test dell’import CSV conteso ora verifica l’acquisizione effettiva del lock MySQL prima del commit(). In caso di errore, mostra durata d’attesa, nome del lock e titolare. Il test rilascia il lock e chiude la connessione bloccante.

Changes

Test dell’import CSV conteso

Layer / File(s) Summary
Precondizione, diagnostica e rilascio del lock
tests/emeroteca-412.unit.php
Il test acquisisce il lock con un ciclo di retry e verifica che sia stato ottenuto. La diagnostica include durata d’attesa, nome del lock e titolare corrente. Il rilascio riusa il nome del lock già escapato.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to 0c2fc

The PR improves lock-contention test reliability and diagnostics without introducing an identified merge-blocking risk.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Il titolo descrive con precisione la modifica principale: il test verifica la precondizione del lock di importazione invece di presumerla.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/import-lock-precondition

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

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

@fabiodalez-dev
fabiodalez-dev merged commit 64d4e05 into main Sep 14, 2026
34 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.

1 participant