Skip to content

test(index): make createContentIndexAndDelete index-count assertion phase-aware (#36501)#36517

Open
fabrizzio-dotCMS wants to merge 2 commits into
mainfrom
issue-36501-fix-createcontentindex-phase-aware
Open

test(index): make createContentIndexAndDelete index-count assertion phase-aware (#36501)#36517
fabrizzio-dotCMS wants to merge 2 commits into
mainfrom
issue-36501-fix-createcontentindex-phase-aware

Conversation

@fabrizzio-dotCMS

Copy link
Copy Markdown
Member

Proposed Changes

Fixes one of the phase-induced test failures tracked in #36501 (found while running the MainSuite battery under an ES→OS migration phase — see #36320).

ContentletIndexAPIImplTest.createContentIndexAndDelete asserted an exact single-store delta after creating a working + live index:

assertEquals((long) oldIndices + 2, newIndices);

Under a dual-write migration phase (-Dopensearch.phase=1/2), createContentIndex fans out to both the ES and OS stores, so listDotCMSIndices() reports two physical entries per logical index and the delta is +4 — failing with expected:<158> but was:<160>.

Fix

Assert the two created indices are present (already checked via foundWorking/foundLive) and that the total grew by at least two, instead of a single-store exact delta:

assertTrue("expected at least two new indices, delta was " + (newIndices - oldIndices),
        newIndices >= oldIndices + 2);

Phase-agnostic: passes in phase 0 (ES only) and under dual-write.

Verification

Ran under phase 1 locally (-Dopensearch.phase=1 -Dit.test=ContentletIndexAPIImplTest): Tests run: 16, Failures: 0, Errors: 0 ✅ (was 1 failure before).

Refs #36501 (tracking), #36320 (ContentletIndexAPIImplTest phase-aware decoupling — this assertion is item #4 there).

…hase-aware (#36501)

ContentletIndexAPIImplTest.createContentIndexAndDelete asserted an exact
single-store delta (oldIndices + 2) after creating a working and live index.
Under a dual-write migration phase (opensearch.phase=1/2) createContentIndex
fans out to both the ES and OS stores, so listDotCMSIndices() reports two
physical entries per logical index and the delta is +4, failing the assertion
(observed "expected:<158> but was:<160>").

Assert the two created indices are present (already checked) and that the total
grew by at least two, instead of a single-store exact delta. Passes in phase 0
(ES only) and under dual-write phases.

Refs #36320 (ContentletIndexAPIImplTest phase-aware decoupling).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

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


Claude finished @fabrizzio-dotCMS's task in 1m 4s —— View job


PR Review — phase-aware createContentIndexAndDelete assertion

  • Read the diff (git diff of the two PR commits)
  • Verified IndexTag.strip API and semantics
  • Reviewed for bugs, conventions, and test correctness

New Issues

No issues found.

The change is a test-only fix and is correct:

  • newIndices >= oldIndices + 2 (line 354–355) — right call. The exact +2 delta was single-store; under dual-write (opensearch.phase=1/2) each logical index produces two physical entries (+4). The >= lower bound is phase-agnostic and holds in phase 0 (ES only, exact +2) and under dual-write (+4). Presence of the two specific indices is still asserted exactly via foundWorking/foundLive, so the weaker count assertion doesn't lose meaningful coverage.
  • IndexTag.strip(index) (lines 339, 378) — correct use of the sanctioned helper. IndexTag.java:216 documents it as safe on untagged names (returns unchanged), so phase 0 names still match, and .os-tagged names in phases 1/2/3 are normalized before comparison. This respects the "only IndexTag may manipulate the marker" invariant in the enum's Javadoc — no ad-hoc .endsWith(".os").
  • Comments accurately describe the phase-3 OS-only behavior and the dual-write fan-out.

Note (non-blocking)

The final assertion at line 390, assertEquals(oldIndices, newIndices) after the deletes, is unchanged by this PR and remains an exact-equality check. It's phase-safe only if indexAPI.delete(...) fans out and removes both physical entries per logical index under dual-write. That's outside this diff and tracked separately under #36320 — flagging only for awareness, not as a defect in this PR.

Looks good to merge. ✅
· issue-36501-fix-createcontentindex-phase-aware

…erant (all phases) (#36501)

The index-count assertion was made phase-tolerant in the previous commit, but the
presence checks (foundWorking/foundLive) still compared against the raw index name
("working_<ts>"). Under OS-only phase 3, listDotCMSIndices() returns only the
.os-tagged names (OSIndexAPIImpl.listIndices strips the cluster prefix but not the
tag), so the raw-name equals() never matched and assertTrue(foundWorking) failed.
In phases 1/2 it passed only incidentally because the ES provider also contributes
the untagged name.

Strip the vendor tag with IndexTag.strip(index) before comparing, so the presence
checks match the logical name in every phase.

Verified createContentIndexAndDelete under phase 2 and phase 3 (1/0/0 each);
phases 0 and 1 already green. Now passes in all four phases.

Refs #36501, #36320.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@fabrizzio-dotCMS
fabrizzio-dotCMS added this pull request to the merge queue Jul 13, 2026
@mergify

mergify Bot commented Jul 13, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants