test(worker): await terminal retry completion before asserting - #454
test(worker): await terminal retry completion before asserting#454seonghobae wants to merge 4 commits into
Conversation
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough비동기 재시도 테스트가 재시도 횟수와 변환 작업의 Changes변환 재시도 검증
Estimated code review effort: 1 (Trivial) | ~3 minutes Merge Risk: ⚪ Minimal · up to This localized test-only change removes an asynchronous assertion race without changing production behavior; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
|
@opencode-agent review Review exact head Review read-only; preserve independent reviewer identity. Do not mutate the branch, synthesize author approval, weaken protection, or merge. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
expected_head='23ba9a34e24d62de7974a6e1707dc53f6f69b4ac'
base='eb0a32e87e7470469c8fa0f8c67d9583654bef57'
repo='ContextualWisdomLab/clearfolio'
pr='454'
echo '== Checked-out commit =='
git rev-parse HEAD
git status --short
echo '== Commit identity =='
git show -s --format='%H%n%P%n%s' "$expected_head"
echo '== Changed paths and diff check =='
git diff --name-status "$base" "$expected_head"
git diff --check "$base" "$expected_head"
echo '== Exact diff =='
git diff --no-ext-diff --unified=80 "$base" "$expected_head" -- src/test/java/com/clearfolio/viewer/service/DefaultConversionWorkerTest.java
echo '== Test file outline =='
ast-grep outline src/test/java/com/clearfolio/viewer/service/DefaultConversionWorkerTest.java --items all --view expanded
echo '== Retry test and synchronization usages =='
rg -n -C 12 -e 'workerReschedulesWhenRetryAtIsInTheFuture' -e '\bawait\b' -e 'Thread\.sleep|TimeUnit\..*sleep' src/test/java/com/clearfolio/viewer/service/DefaultConversionWorkerTest.java
echo '== Pull request metadata =='
gh pr view "$pr" --repo "$repo" --json number,headRefOid,baseRefOid,changedFiles,mergeStateStatus,reviewDecision,statusCheckRollup,reviews
echo '== Review threads =='
gh api graphql -F owner='ContextualWisdomLab' -F name='clearfolio' -F number="$pr" -f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) {
nodes {
isResolved
isOutdated
path
line
comments(first: 20) {
nodes { author { login } body url }
}
}
}
}
}
}'Length of output: 29071
Finding: Add: assertNull(job.getRetryAt());The new The exact-head diff changes one test file only. You are interacting with an AI system. |
|
Exact-current-head independent review request for CI, Security Scan, SAST Semgrep, and fuzz are terminal-success; unresolved inline review threads are zero. The change is test synchronization only and leaves production behavior unchanged. @opencode-agent review Review this exact head read-only. Do not mutate the branch, synthesize author approval, weaken protection, or merge. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
head='23ba9a34e24d62de7974a6e1707dc53f6f69b4ac'
base='eb0a32e87e7470469c8fa0f8c67d9583654bef57'
repo='ContextualWisdomLab/clearfolio'
pr='454'
file='src/test/java/com/clearfolio/viewer/service/DefaultConversionWorkerTest.java'
echo '== Exact commit =='
git rev-parse HEAD
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$head"
git merge-base --is-ancestor "$base" "$head" && echo 'base-is-ancestor=yes'
echo '== Changed paths and whitespace =='
git diff --name-status "$base" "$head"
git diff --check "$base" "$head"
echo '== Target file diff =='
git diff --no-ext-diff --unified=100 "$base" "$head" -- "$file"
echo '== Target file structure =='
ast-grep outline "$file" --items all --view expanded
echo '== Retry test, await predicates, assertions, and sleep references =='
rg -n -C 15 \
-e 'workerReschedulesWhenRetryAtIsInTheFuture' \
-e '\bawait\b' \
-e 'assert(Equals|Null|That)' \
-e 'Thread\.sleep|TimeUnit\..*sleep' \
"$file"
echo '== PR metadata and check status =='
gh pr view "$pr" --repo "$repo" \
--json number,headRefOid,baseRefOid,changedFiles,mergeStateStatus,reviewDecision,statusCheckRollup \
--jq '{number,headRefOid,baseRefOid,changedFiles,mergeStateStatus,reviewDecision,statusCheckRollup:[.statusCheckRollup[]|{name,conclusion,status}]}'
echo '== Review thread counts =='
gh api graphql -F owner='ContextualWisdomLab' -F name='clearfolio' -F number="$pr" -f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) {
nodes { isResolved isOutdated path line }
}
}
}
}' --jq '
[.data.repository.pullRequest.reviewThreads.nodes[]] as $t |
{
total: ($t|length),
unresolved: ([$t[]|select(.isResolved|not)]|length),
unresolvedCurrent: ([$t[]|select((.isResolved|not) and (.isOutdated|not))]|length),
unresolvedCurrentThreads: [$t[]|select((.isResolved|not) and (.isOutdated|not))|{path,line}]
}'Length of output: 52176
Finding — Add this assertion after the success-state assertion: assertNull(job.getRetryAt());The new The diff changes one test file only.
|
|
Exact-current-head independent review request for Freshly rechecked: CI, Security Scan, SAST Semgrep, and fuzz are terminal-success; mergeable is true; unresolved inline review threads are zero. This is a one-file test-synchronization repair with no production-code change. Review this exact head only; predecessor evidence does not transfer. @coderabbitai review |
Rate Limit Exceeded
|
1 participant
Objective
Remove a deterministic asynchronous test race that can fail otherwise-correct worker and delivery-receipt PRs. The retry test previously unblocked as soon as the conversion task incremented its attempt counter, but
DefaultConversionWorkerpublishesSUCCEEDEDonly after that converter call returns and the state store completes its terminal transition.Exact test-first repair
eb0a32e87e7470469c8fa0f8c67d9583654bef57;23ba9a34e24d62de7974a6e1707dc53f6f69b4ac;DefaultConversionWorkerTest.javaonly;31879285024: terminal-success;mvn -B --no-transfer-progress verify, authoritative test-report verification, andgit diff --check, then removed itself before publishing this candidate.Contract
workerReschedulesWhenRetryAtIsInTheFuturenow waits until both the converter has executed once and the observed job state isSUCCEEDED. Assertions still require exactly one converter call, one attempt, terminal success, cleared retry time, and the expected converted resource path. Production behavior is unchanged.Scope boundary
Test synchronization only. This does not weaken timeouts, add sleeps, retry assertions after failure, modify worker/state-store code, change scheduling, or alter delivery-receipt semantics.
Dependency outcome
PR #373 failed only because this shared test observed
PROCESSINGbetween converter execution and terminal publication. After this repair reaches protectedmain, #373 must update to the new exact base and regenerate all checks/reviews; predecessor evidence does not transfer.Merge gate
Keep this exact head unchanged. Require current-head CI, Security Scan, SAST Semgrep, fuzz where emitted, zero valid unresolved findings, and qualifying independent non-author approval before protected merge.
Summary by CodeRabbit