Skip to content

test(postman): assert active import jobs against their state, not timing (#37887) - #37912

Merged
fabrizzio-dotCMS merged 2 commits into
mainfrom
37887-postman-active-jobs-assertions
Oct 6, 2026
Merged

fabrizzio-dotCMS merged 2 commits into
mainfrom
37887-postman-active-jobs-assertions

Conversation

@fabrizzio-dotCMS

@fabrizzio-dotCMS fabrizzio-dotCMS commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

Closes #37887

Problem

ContentImportResource → TestGetStatus → Get all active Jobs asserted that no active job had startedAt or completedAt. The job processor runs concurrently with the collection, so a freshly created job can already be RUNNING (with startedAt) when the request lands — which is what failed run 37020786581. The active list (PostgresJobQueue.getActiveJobs) also includes FAILED and ABANDONED jobs, which carry completedAt, so that assertion had the same latent problem.

Change

Only the "Get all active Jobs" test script changes; no server code.

  • state must be one of the states getActiveJobs returns: PENDING, RUNNING, FAILED, ABANDONED, CANCEL_REQUESTED, CANCELLING.
  • Timestamps are asserted against the reported state:
    • PENDING → no startedAt, no completedAt
    • RUNNING → startedAt present, no completedAt
    • FAILED / ABANDONED → completedAt present
    • cancel states → timestamps optional, validated when present (a pending job can be cancelled)
  • "jobId is present" now also accepts a job that already left the active list, but only if GET /api/v1/content/_import/{jobId} reports SUCCESS.

Verification

  • Collection run 10 times in a row locally against the test image: 182 assertions, 0 failures each time.
  • Those runs never hit the race, so the race branches were forced with canned responses (Newman + a mock server), old script vs new:
Scenario old new
New job already RUNNING with startedAt (the CI failure) fail pass
FAILED job with completedAt in the active list fail pass
New job left the list, status SUCCESS fail pass
New job left the list, status FAILED fail fail
RUNNING without startedAt pass fail
PENDING with startedAt fail fail
SUCCESS in the active list fail fail
All PENDING pass pass

🤖 Generated with Claude Code

This PR fixes: #37887

…ing (#37887)

The "Get all active Jobs" request asserted that no active job had startedAt or
completedAt. The job processor runs concurrently, so a job can be RUNNING by the
time the request lands, and the active list also includes FAILED and ABANDONED
jobs, which carry completedAt.

Timestamps are now checked against each job's reported state, the state must be
one of the states getActiveJobs returns, and a newly created job missing from the
active list is accepted only if its status endpoint reports SUCCESS.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @fabrizzio-dotCMS's task in 11s —— View job


I'll analyze this and get back to you.

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

⚠️ AI review failed

Claude did not produce a review — the backend call errored before generating any output (provider: anthropic-bedrock, model: global.anthropic.claude-sonnet-5). This usually means the model has no Bedrock access grant in the target account, or the model ID is invalid — not a problem with this PR.

Run: #37485927997

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

⚠️ AI review failed

Claude did not produce a review — the backend call errored before generating any output (provider: anthropic-bedrock, model: global.anthropic.claude-sonnet-5). This usually means the model has no Bedrock access grant in the target account, or the model ID is invalid — not a problem with this PR.

Run: #37485927681

@sfreudenthaler sfreudenthaler added the PR : dotbot review Trigger dotbot AI code review and the post-merge QA test plan label Oct 6, 2026
Resolve the server URL with the same baseUrl fallback the collection pre-request
script uses, and assert the lookup returned 200 before reading entity.state so an
error response fails with a readable message instead of a TypeError.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

dotbot code review:

  • Reviewer: meta/muse-spark-1.3 (medium)
  • Overall: patch is correct
  • New findings this run: 0
  • Prior unresolved dotbot findings still relevant: 0
  • Active findings total: 0

Test-only change. ACTIVE_STATES exactly matches PostgresJobQueue.getActiveJobs filter and state-conditional timestamp assertions remove the timing race. Both prior fallback-lookup hardening items are verified present in HEAD.

Tip: comment with "/dotbot address comments" to attempt automated fixes for unresolved review threads.

reviewed by dotbot · meta/muse-spark-1.3 · medium

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

dotbot code review:

  • Reviewer: ~z-ai/glm-latest (medium)
  • Overall: patch is correct
  • New findings this run: 0
  • Prior unresolved dotbot findings still relevant: 0
  • Active findings total: 0

Test-only change to a Postman collection. ACTIVE_STATES exactly matches the states filtered by PostgresJobQueue.getActiveJobs (PENDING, RUNNING, FAILED, ABANDONED, CANCEL_REQUESTED, CANCELLING), and the state-conditional timestamp assertions match the Job lifecycle transitions in AbstractJob (markAsRunning always sets startedAt; markAsFailed/markAsAbandoned always set completedAt). Both prior review findings (serverURL fallback and res.code guard in the fallback lookup) are present in HEAD. No new issues found.

Tip: comment with "/dotbot address comments" to attempt automated fixes for unresolved review threads.

reviewed by dotbot · ~z-ai/glm-latest · medium

@dotCMS-Machine-User dotCMS-Machine-User left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

✅ dotbot review: all reviewer models (meta/muse-spark-1.3, ~z-ai/glm-latest) agree — patch is correct.

approved automatically by dotbot

@ihoffmann-dot ihoffmann-dot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Non-blocking suggestion: ContentImportResource.postman_collection.json, test "jobId is present in the response or already finished successfully"

When the job is not in the active list, the outer pm.test returns immediately and the real SUCCESS assertion runs in a nested pm.test inside the async pm.sendRequest callback. The outer test is always green, and a failed or cancelled job is only reported by the nested test, which may register after the outer one completes.

Consider moving the fallback lookup out of the outer test into its own top-level pm.sendRequest + pm.test, so a single, clearly named test carries the pass/fail result.

The rest looks good: ACTIVE_STATES matches PostgresJobQueue.getActiveJobs, and asserting timestamps against the reported state removes the timing flakiness.

@fabrizzio-dotCMS
fabrizzio-dotCMS added this pull request to the merge queue Oct 6, 2026
Merged via the queue into main with commit b28a2a6 Oct 6, 2026
87 checks passed
@fabrizzio-dotCMS
fabrizzio-dotCMS deleted the 37887-postman-active-jobs-assertions branch October 6, 2026 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area : Backend PR changes Java/Maven backend code PR : dotbot review Trigger dotbot AI code review and the post-merge QA test plan

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

[Flakey Test] Postman ContentImportResource asserts no startedAt on a job that may already be running

4 participants