Skip to content

test(import): use unique content-type variables in ImportUtilTest relationship tests (#36501) - #36518

Merged
fabrizzio-dotCMS merged 1 commit into
mainfrom
issue-36501-fix-importutiltest-unique-ct-vars
Jul 13, 2026
Merged

test(import): use unique content-type variables in ImportUtilTest relationship tests (#36501)#36518
fabrizzio-dotCMS merged 1 commit into
mainfrom
issue-36501-fix-importutiltest-unique-ct-vars

Conversation

@fabrizzio-dotCMS

@fabrizzio-dotCMS fabrizzio-dotCMS commented Jul 10, 2026

Copy link
Copy Markdown
Member

Proposed Changes

Fixes the phase-induced ImportUtilTest failures tracked in #36501 (found while running MainSuite1a under an ES→OS migration phase — see #36320).

Root cause

The relationship / self-related import tests hardcoded shared content-type variables ("parentContentType", "childContentType", "selfRelatedType") and deleted them in a finally block. Content-type deletion disposes the variable asynchronously; under a dual-write phase (-Dopensearch.phase=1) that disposal lags, so the next test creating the same fixed variable hits doesTypeWithVariableExist() == true and fails with:

IllegalArgumentException: Invalid content type variable: parentContentType

cascading across ~7 relationship tests in MainSuite1a. The baseline (phase 0) passes because disposal completes promptly.

Fix

Append a per-call timestamp to the variable so each test is isolated regardless of async cleanup timing — mirroring sibling tests in the same file that already use "parentContentType" + new Date().getTime(). Test-isolation robustness only; no product change.

Verification

Ran under phase 1 locally: the Invalid content type variable: parentContentType errors are eliminated (0 remaining, was 7).

⚠️ Note — separate pre-existing issue surfaced (not addressed here)

With the cascade removed, ImportUtilTest now runs far enough to reveal 11 unrelated failures in the unique-field tests (testingImportWithUniqueFields, importFile_fails_when_twoLinesHaveSameUniqueKeys, …). These are i18n message-resolution failures — LanguageUtil.get(user, "were-created" / "contains-duplicate-values-for-structure-unique-field") returns the raw key instead of the resolved text, even though the keys exist in Language.properties. This is phase-agnostic (no ES/OS involvement) and independent of this change; it was previously masked by the parentContentType setup errors. Filing/investigating separately under #36501. It may be a local-env artifact (the import message code merged to main through green CI).

Refs #36501.

This PR fixes: #36501

…ationship tests (#36501)

The relationship / self-related import tests hardcoded shared content-type
variables ("parentContentType", "childContentType", "selfRelatedType") and
deleted them in a finally block. Content-type deletion disposes the variable
asynchronously; under a dual-write migration phase (opensearch.phase=1) that
disposal lags, so the next test creating the same fixed variable hits
doesTypeWithVariableExist() == true and fails with
"IllegalArgumentException: Invalid content type variable: parentContentType"
(cascading across ~7 relationship tests in MainSuite1a).

Append a per-call timestamp to the variable so each test is isolated regardless
of async cleanup timing — mirroring the existing sibling tests that already use
"parentContentType" + new Date().getTime(). Test-isolation robustness; no
product change.

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 53s —— View job


Code Review

  • Read PR diff and changed test file
  • Verify variable references are self-consistent
  • Assess correctness of the isolation fix

Review

This is a clean, test-only change. All 8 call sites replace the hardcoded content-type variable ("parentContentType" / "childContentType" / "selfRelatedType") with a per-call timestamp suffix (+ new Date().getTime()), matching the pattern already used by sibling tests in the same file.

Correctness verified:

  • Relationship resolution uses the dynamic variable via parentContentType.variable() / childContentType.variable() (e.g. ImportUtilTest.java:1627,1631, 1703), not the literal string, so appending a timestamp doesn't break any lookups.
  • The relationship field variable ("testRelationship") is per-content-type scoped and unchanged — no collision concern.
  • finally-block cleanup deletes by object reference (contentTypeApi.delete(parentContentType)), so it's unaffected by the variable name change.
  • Date is already imported and used elsewhere in this file.

The root-cause reasoning holds: content-type disposal is async, and under opensearch.phase=1 the lag causes doesTypeWithVariableExist() to still return true for the fixed variable, tripping IllegalArgumentException: Invalid content type variable. Timestamp-suffixing sidesteps the shared-name collision regardless of cleanup timing.

Minor (non-blocking) observation: new Date().getTime() gives millisecond resolution. If two of these tests were ever run within the same millisecond they could still collide — but JUnit runs them sequentially with content-type creation between them (far more than 1ms), and this simply mirrors the existing convention in the file, so it's fine as-is. System.nanoTime() would be marginally more robust but is not worth diverging from the established pattern.

The separate i18n message-resolution failures noted in the PR description are correctly scoped out of this change.

New Issues

No issues found.
issue-36501-fix-importutiltest-unique-ct-vars

@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

@fabrizzio-dotCMS
fabrizzio-dotCMS added this pull request to the merge queue Jul 13, 2026
Merged via the queue into main with commit 7ca3acd Jul 13, 2026
62 checks passed
@fabrizzio-dotCMS
fabrizzio-dotCMS deleted the issue-36501-fix-importutiltest-unique-ct-vars branch July 13, 2026 17:52
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.

bug(index): product fixes & findings from phased (ES→OS) integration runs

2 participants