Skip to content

E2E: QA add acceptance tests published querying parity#23047

Open
andr317c wants to merge 6 commits into
mainfrom
v18/QA/add-acceptance-tests-published-querying-parity
Open

E2E: QA add acceptance tests published querying parity#23047
andr317c wants to merge 6 commits into
mainfrom
v18/QA/add-acceptance-tests-published-querying-parity

Conversation

@andr317c
Copy link
Copy Markdown
Contributor

@andr317c andr317c commented Jun 2, 2026

No description provided.

andr317c and others added 5 commits June 1, 2026 10:17
…ished querying parity

Covers PR #22622 (V13/V17 published querying parity) at the E2E level:
- a descendant is not routable in a culture where an ancestor is unpublished
- a descendant still renders in cultures where the ancestor chain is published
- Parent/Ancestors() return ancestors that are unpublished in the requested culture

Adds reusable variant-branch, per-culture publish/unpublish, culture URL and
path-domain helpers to support culture-variant rendering scenarios.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 2, 2026 18:51
@claude
Copy link
Copy Markdown

claude Bot commented Jun 2, 2026

Claude finished @andr317c's task in 3m 10s —— View job


PR Review: E2E tests for published querying parity

Overall verdict: Approve with suggestions. The tests cover meaningful behavior (culture publish status querying parity), the new helper methods are clean and reusable, and the setup/teardown is well structured. Two findings worth addressing before merge.


Summary of findings

Severity File Finding
Medium package.json:26 Typo RenderingContentWIthCulturePublishStatus (capital I) — won't match the file on Linux/CI
Medium package.json:26 smokeTest now runs a single spec instead of all @smoke-tagged tests — old behavior moved to smokeTester without announcing the rename
Low RenderingContentWithCulturePublishStatus.spec.ts:81 Second test asserts the published child IS present but doesn't verify the grandchild is absent — the stated exclusion of the grandchild is unverified
Low RenderingContentWithCulturePublishStatus.spec.ts:50 domainsData from getDomains is mutated in-place; DocumentDomainBuilder would be safer and consistent with the rest of the codebase

Positive notes

  • createVariantDocumentWithParent, publishWithCultures, unpublish, getDocumentUrlByCulture are clean, focused additions to the helper API.
  • createVariantDocumentTypeWithTemplateAndAllowedChildNode nicely composes the existing builder pattern and correctly defaults isAllowedAsRoot to false.
  • The three-level content tree (root → published child + unpublished child → grandchild) is exactly the right setup to test the ancestor-publish-status behavior.
  • Cleanup in afterEach is thorough — document type and template cleanup is included alongside the content cleanup.

Labels applied: area/test, category/test-automation

@@ -23,7 +23,8 @@
"testSqlite": "npm run build && npx playwright test DefaultConfig --grep-invert \"Users\"",
"all": "npm run build && npx playwright test",
"createTest": "node createTest.js",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[Medium] Typo in smokeTest script: RenderingContentWIthCulturePublishStatus has an uppercase I in "WIth", but the actual spec file is named RenderingContentWithCulturePublishStatus.spec.ts. On Linux (where CI runs), filesystem lookups are case-sensitive, so this script will likely find zero tests and exit silently.

Suggestion: change RenderingContentWIthCulturePublishStatusRenderingContentWithCulturePublishStatus.

Fix this →

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds new Playwright acceptance coverage for culture-variant published/unpublished descendant rendering, plus a few helper APIs to create/publish/unpublish variant documents and document types.

Changes:

  • Added E2E spec verifying descendant rendering parity when an ancestor is unpublished in a specific culture.
  • Extended acceptance-test API helpers with variant document/document type creation and culture-scoped publish/unpublish/url utilities.
  • Updated acceptance-test package.json scripts (currently with a broken/behavior-changing smokeTest entry).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
tests/Umbraco.Tests.AcceptanceTest/tests/DefaultConfig/RenderingContent/RenderingContentWithCulturePublishStatus.spec.ts New E2E test validating culture-specific publish state affects descendant querying/rendering.
tests/Umbraco.Tests.AcceptanceTest/package.json Adjusts smoke scripts (currently introduces a likely typo + changes smoke semantics).
tests/Umbraco.Tests.AcceptanceTest/lib/helpers/DocumentTypeApiHelper.ts Adds helper to create variant doc types with template + allowed child configuration.
tests/Umbraco.Tests.AcceptanceTest/lib/helpers/DocumentApiHelper.ts Adds helpers for publish/unpublish by cultures, culture-specific URL lookup, and variant document creation.

Comment on lines +26 to +27
"smokeTest": "npm run build && npx playwright test RenderingContentWIthCulturePublishStatus",
"smokeTester": "npm run build && npx playwright test DefaultConfig --grep \"@smoke\"",
Comment on lines +184 to +190
async getDocumentUrlByCulture(id: string, culture: string) {
const response = await this.api.get(this.api.baseUrl + '/umbraco/management/api/v1/document/urls?id=' + id);
const urls = await response.json();
const urlInfo = urls[0].urlInfos.find(info => info.culture === culture);

return urlInfo ? urlInfo.url : null;
}
Comment on lines +1533 to +1540
async createVariantDocumentWithParent(documentTypeId: string, templateId: string, name: string, cultures: string[], parentId?: string) {
const documentBuilder = new DocumentBuilder()
.withDocumentTypeId(documentTypeId)
.withTemplateId(templateId);

if (parentId) {
documentBuilder.withParentId(parentId);
}
Comment on lines +14 to +17
const rootContentName = 'RootContent';
const publishedChildName = 'PublishedChild';
const unpublishedChildName = 'UnpublishedChild';
const grandchildContentName = 'Grandchild';
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jun 3, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants