From bf4b296d283fb5f535d331b239779392725e0a67 Mon Sep 17 00:00:00 2001 From: MortenFriisSiteImprove Date: Wed, 16 Sep 2026 08:39:41 +0200 Subject: [PATCH 1/2] Open Level A before checking the prepublish image issue --- tests/browser/result-navigation.spec.js | 21 ++++++++++++++++----- tests/live/result-view.mjs | 18 ++++++++++++++---- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/tests/browser/result-navigation.spec.js b/tests/browser/result-navigation.spec.js index f4d460e..b0134db 100644 --- a/tests/browser/result-navigation.spec.js +++ b/tests/browser/result-navigation.spec.js @@ -1,12 +1,16 @@ const { test, expect } = require('@playwright/test'); -async function fixture(page, expanded, issue) { +async function fixture(page, expanded, issue, levelExpanded = false) { await page.setContent(` -
${issue ? '

Image missing a text alternative

' : '

No accessibility issues

'}
+
+ + +
${issue ? '

Image missing a text alternative

' : '

No accessibility issues

'}
+
`); } -test('opens collapsed accessibility results to expose the image alternative issue', async ({ page }) => { +test('opens Accessibility then Level A to expose the image alternative issue', async ({ page }) => { const { openAccessibilityResults, resultViewState } = await import('../live/result-view.mjs'); await fixture(page, false, true); await openAccessibilityResults(page); @@ -15,8 +19,15 @@ test('opens collapsed accessibility results to expose the image alternative issu test('keeps expanded corrected results open and does not manufacture an issue', async ({ page }) => { const { openAccessibilityResults, resultViewState } = await import('../live/result-view.mjs'); - await fixture(page, true, false); + await fixture(page, true, false, true); await openAccessibilityResults(page); - await expect(page.locator('section')).toBeVisible(); + await expect(page.locator('#level-a')).toBeVisible(); expect(await resultViewState(page)).toMatchObject({ imageIssuePresent: false, imageIssueVisible: false, resultAlertVisible: false }); }); + +test('opens Level A when Accessibility is already open', async ({ page }) => { + const { openAccessibilityResults } = await import('../live/result-view.mjs'); + await fixture(page, true, true); + await openAccessibilityResults(page); + await expect(page.locator('#level-a')).toBeVisible(); +}); diff --git a/tests/live/result-view.mjs b/tests/live/result-view.mjs index 8695e93..d5505ba 100644 --- a/tests/live/result-view.mjs +++ b/tests/live/result-view.mjs @@ -5,13 +5,23 @@ const category = overlay => overlay.getByRole('button', { name: /^Accessibility\ .or(overlay.getByRole('tab', { name: /^Accessibility\b/i })) .or(overlay.getByText('Accessibility', { exact: true })).filter({ visible: true }).first(); +const levelA = overlay => overlay.getByRole('button', { name: /^Level A(?:\s|$)/ }) + .or(overlay.getByRole('tab', { name: /^Level A(?:\s|$)/ })) + .or(overlay.getByText('Level A', { exact: true })).filter({ visible: true }).first(); + export async function openAccessibilityResults(overlay) { const issue = overlay.getByText(imageAlternativeRule.label, { exact: true }).filter({ visible: true }); if (await issue.count()) return; - const control = category(overlay); - await expect(control).toBeVisible(); - if (await control.getAttribute('aria-expanded') !== 'true' - && await control.getAttribute('aria-selected') !== 'true') await control.click(); + if (!await levelA(overlay).isVisible()) { + const control = category(overlay); + await expect(control).toBeVisible(); + if (await control.getAttribute('aria-expanded') !== 'true' + && await control.getAttribute('aria-selected') !== 'true') await control.click(); + } + const level = levelA(overlay); + await expect(level).toBeVisible(); + if (await level.getAttribute('aria-expanded') !== 'true' + && await level.getAttribute('aria-selected') !== 'true') await level.click(); } export async function resultViewState(overlay) { From 291d57a5fcf07ef3af255187ba356661ef4be8f8 Mon Sep 17 00:00:00 2001 From: MortenFriisSiteImprove Date: Wed, 16 Sep 2026 08:46:06 +0200 Subject: [PATCH 2/2] Assert block handling produces no browser or server errors --- docs/testing.md | 4 +++ tests/CmsHost/BlockErrorLog.cs | 26 +++++++++++++++++++ tests/CmsHost/RegressionBlock.cs | 10 +++++++ tests/CmsHost/Startup.cs | 20 ++++++++++++++ tests/Plugin.Tests/BlockErrorLogTests.cs | 21 +++++++++++++++ tests/Plugin.Tests/Plugin.Tests.csproj | 1 + tests/browser/prepublish.spec.js | 11 +++++--- tests/cms/cms.spec.ts | 33 ++++++++++++++++++++++++ 8 files changed, 123 insertions(+), 3 deletions(-) create mode 100644 tests/CmsHost/BlockErrorLog.cs create mode 100644 tests/CmsHost/RegressionBlock.cs create mode 100644 tests/Plugin.Tests/BlockErrorLogTests.cs diff --git a/docs/testing.md b/docs/testing.md index 1b657dd..f4eef77 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -108,3 +108,7 @@ This is a compatibility sample, not a claim to cover every version in the packag A separate required job runs the published 4.3.3 package and then the candidate on `cms12-current`, keeping the same SQL database and application data. The baseline comes from the official Optimizely feed and is verified against a pinned SHA-256 before installation. Both installed DLLs and module ZIPs must match their respective packages. The baseline saves synthetic configuration through its own settings repository and creates published content plus an unpublished draft. After replacement, seeding is disabled. Playwright checks the original settings record, token, API fields, flags, URL mappings, editor login and draft reference/content, then checks the candidate settings UI and preview callback. Published content must remain unchanged. Siteimprove responses remain controlled. This covers one plugin upgrade path, not a CMS upgrade or every historical plugin version. + +### Block error regressions + +Blocks remain unsupported by the plugin. Browser regressions require Page → Block → Page navigation and initial block context to produce no console errors or unhandled exceptions. The packaged CMS suite publishes a real shared block, checks that a direct PageUrl request returns a controlled 400, opens the block and returns to page editing without block PageUrl requests or browser errors. Server error and TypeMismatchException counts must remain unchanged during this scenario on both CMS profiles. Counts are retained without log messages or content. The expected direct 400 is asserted separately from browser traffic. diff --git a/tests/CmsHost/BlockErrorLog.cs b/tests/CmsHost/BlockErrorLog.cs new file mode 100644 index 0000000..adee69e --- /dev/null +++ b/tests/CmsHost/BlockErrorLog.cs @@ -0,0 +1,26 @@ +using Microsoft.Extensions.Logging; + +namespace CmsHost; + +// Retain counts only; exception messages and content never leave the logger. +public sealed class BlockErrorLog : ILoggerProvider +{ + private long errors; + private long typeMismatches; + public object Snapshot() => new { errors = Interlocked.Read(ref errors), typeMismatches = Interlocked.Read(ref typeMismatches) }; + public ILogger CreateLogger(string categoryName) => new Counter(this); + public void Dispose() { } + + private sealed class Counter(BlockErrorLog owner) : ILogger + { + public IDisposable? BeginScope(TState state) where TState : notnull => null; + public bool IsEnabled(LogLevel level) => level >= LogLevel.Warning; + public void Log(LogLevel level, EventId id, TState state, Exception? exception, Func formatter) + { + if (level >= LogLevel.Error) Interlocked.Increment(ref owner.errors); + if (exception?.ToString().Contains("TypeMismatchException", StringComparison.Ordinal) == true + || formatter(state, exception).Contains("TypeMismatchException", StringComparison.Ordinal)) + Interlocked.Increment(ref owner.typeMismatches); + } + } +} diff --git a/tests/CmsHost/RegressionBlock.cs b/tests/CmsHost/RegressionBlock.cs new file mode 100644 index 0000000..905a9ce --- /dev/null +++ b/tests/CmsHost/RegressionBlock.cs @@ -0,0 +1,10 @@ +using EPiServer.Core; +using EPiServer.DataAnnotations; + +namespace CmsHost; + +[ContentType(DisplayName = "Regression block", GUID = "d79aeb73-f761-418b-b3ba-86838db73599")] +public class RegressionBlock : BlockData +{ + public virtual string Text { get; set; } = ""; +} diff --git a/tests/CmsHost/Startup.cs b/tests/CmsHost/Startup.cs index 85c2767..c468faf 100644 --- a/tests/CmsHost/Startup.cs +++ b/tests/CmsHost/Startup.cs @@ -16,6 +16,11 @@ public void ConfigureServices(IServiceCollection services) services.AddDataProtection().PersistKeysToFileSystem(new DirectoryInfo("App_Data/keys")); services.AddCmsAspNetIdentity(); services.AddCms(); + if (Environment.GetEnvironmentVariable("CMS_SITEIMPROVE_MODE") != "live") + { + services.AddSingleton(); + services.AddSingleton(sp => sp.GetRequiredService()); + } services.Configure(o => { o.UpdateDatabaseSchema = true; o.CreateDatabaseSchema = true; }); if (Environment.GetEnvironmentVariable("CMS_SITEIMPROVE_MODE") != "live") { @@ -74,6 +79,21 @@ public void Configure(IApplicationBuilder app) settings.LatestUI, settings.ApiUser, settings.ApiKey, settings.UrlMap } }); }).RequireAuthorization(Constants.SiteImproveAuthorizationPolicy); + if (Environment.GetEnvironmentVariable("CMS_SITEIMPROVE_MODE") != "live") + { + endpoints.MapGet("/test/block-errors", (BlockErrorLog log) => log.Snapshot()) + .RequireAuthorization(Constants.SiteImproveAuthorizationPolicy); + endpoints.MapPost("/test/block", (HttpContext context, EPiServer.IContentRepository repository) => + { + if (context.Request.Headers["X-Cms-Test"] != "block-regression") return Results.NotFound(); + var block = repository.GetDefault(EPiServer.Core.ContentReference.GlobalBlockFolder); + block.Text = "Synthetic block"; + var content = (EPiServer.Core.IContent)block; + content.Name = "Regression block"; + var reference = repository.Save(content, EPiServer.DataAccess.SaveAction.Publish, EPiServer.Security.AccessLevel.NoAccess); + return Results.Ok(new { contentId = reference.ToString() }); + }).RequireAuthorization(Constants.SiteImproveAuthorizationPolicy); + } endpoints.MapGet("/test/ready", () => Seed.Ready ? Results.Ok() : Results.StatusCode(503)); }); } diff --git a/tests/Plugin.Tests/BlockErrorLogTests.cs b/tests/Plugin.Tests/BlockErrorLogTests.cs new file mode 100644 index 0000000..6e05f5f --- /dev/null +++ b/tests/Plugin.Tests/BlockErrorLogTests.cs @@ -0,0 +1,21 @@ +using CmsHost; +using Microsoft.Extensions.Logging; +using System.Text.Json; +using Xunit; + +namespace Plugin.Tests; + +public class BlockErrorLogTests +{ + [Fact] + public void Log_guard_counts_errors_and_type_mismatches_without_retaining_messages() + { + using var provider = new BlockErrorLog(); + var logger = provider.CreateLogger("CMS"); + logger.LogWarning("Ordinary warning"); + Assert.Equal("{\"errors\":0,\"typeMismatches\":0}", JsonSerializer.Serialize(provider.Snapshot())); + logger.LogError(new InvalidOperationException("private content"), "Request failed"); + logger.LogWarning("TypeMismatchException: private content"); + Assert.Equal("{\"errors\":1,\"typeMismatches\":1}", JsonSerializer.Serialize(provider.Snapshot())); + } +} diff --git a/tests/Plugin.Tests/Plugin.Tests.csproj b/tests/Plugin.Tests/Plugin.Tests.csproj index cf0585d..3bd435e 100644 --- a/tests/Plugin.Tests/Plugin.Tests.csproj +++ b/tests/Plugin.Tests/Plugin.Tests.csproj @@ -8,6 +8,7 @@ true + diff --git a/tests/browser/prepublish.spec.js b/tests/browser/prepublish.spec.js index 041bb9f..56dbd7c 100644 --- a/tests/browser/prepublish.spec.js +++ b/tests/browser/prepublish.spec.js @@ -17,6 +17,9 @@ test.afterEach(async () => { }); async function setup(page, options = {}) { + const browserErrors = []; + page.on('pageerror', error => browserErrors.push(error.name)); + page.on('console', message => { if (message.type() === 'error') browserErrors.push('console error'); }); const delivery = await listen((req, res) => { res.setHeader('Content-Type', 'text/html'); res.end('Published
PUBLISHED CONTENT
'); @@ -66,7 +69,7 @@ async function setup(page, options = {}) { }, { delivery, options }); await page.addScriptTag({ path: process.env.SITEIMPROVE_TEST_SCRIPT || path.resolve(__dirname, '../../SiteImprove.Optimizely.Plugin/modules/_protected/SiteImprove.Optimizely.Plugin_files/1.0.5/ClientResources/Scripts/siteimprove.js') }); await page.waitForFunction(() => window._si.some(command => command[0] === 'registerPrepublishCallback')); - return { cms, delivery }; + return { cms, delivery, browserErrors }; } async function capture(page) { @@ -146,7 +149,7 @@ for (const mode of ['cross-origin', 'redirect', 'xfo', 'csp']) { for (const event of ['/epi/shell/context/changed', 'epi/shell/context/request']) { test(`context: Page to Block to Page avoids Block URL requests through ${event}`, async ({ page }) => { - const { cms, delivery } = await setup(page); + const { cms, delivery, browserErrors } = await setup(page); const result = await page.evaluate(async event => { const handler = window.subscriptions[event]; const flush = () => new Promise(resolve => setTimeout(resolve, 0)); @@ -178,6 +181,7 @@ for (const event of ['/epi/shell/context/changed', 'epi/shell/context/request']) // This verifies capture resumes on B; it does not test CMS rendering itself. await page.frame({ name: 'sitePreview' }).goto(`${cms}/preview?id=43&language=da`); expect(await capture(page)).toEqual({ status: 'document', text: 'SECOND DRAFT', url: `${cms}/preview?id=43&language=da` }); + expect(browserErrors).toEqual([]); }); } @@ -196,7 +200,7 @@ test('context: absent or incomplete context is ignored without throwing or reque }); test('starting on a Block still allows the first Page to initialize once', async ({ page }) => { - const { delivery } = await setup(page, { initialContext: { id: 'block-99', capabilities: { isPage: false } } }); + const { delivery, browserErrors } = await setup(page, { initialContext: { id: 'block-99', capabilities: { isPage: false } } }); expect(await page.evaluate(() => window.requests.filter(request => request.url === '/api/pageUrl'))).toEqual([]); await page.evaluate(() => { const context = { id: '42_7', language: 'da', capabilities: { isPage: true } }; @@ -206,6 +210,7 @@ test('starting on a Block still allows the first Page to initialize once', async await expect.poll(() => page.evaluate(() => window._si.filter(command => command[0] === 'input').map(command => command.slice(0, 3)))) .toEqual([['input', `${delivery}/da/pages/42_7`, 'fixture-token']]); expect(await page.evaluate(() => window.requests.filter(request => request.url === '/api/pageUrl').length)).toBe(1); + expect(browserErrors).toEqual([]); }); test('a failed page URL lookup resets the page context and the next Page succeeds', async ({ page }) => { diff --git a/tests/cms/cms.spec.ts b/tests/cms/cms.spec.ts index 1905fc7..375a6b1 100644 --- a/tests/cms/cms.spec.ts +++ b/tests/cms/cms.spec.ts @@ -166,3 +166,36 @@ test('prepublish fixture preserves published content while draft edits persist', await expect(preview.locator('#live-test-image')).toHaveAttribute('alt', 'Blue square for the prepublish test'); expect(await (await page.request.get('/draft-test-page/')).text()).not.toContain(process.env.CMS_DRAFT_FIXED_MARKER!); }); + + +test('shared blocks are rejected without browser or server errors and page editing recovers', async ({ page, evidence }) => { + await login(page); + await selectPage(page, 'First page'); + const snapshot = async () => { + const response = await page.request.get('/test/block-errors'); + expect(response.ok()).toBe(true); + return response.json(); + }; + const before = await snapshot(); + evidence.length = 0; + const created = await page.request.post('/test/block', { headers: { 'X-Cms-Test': 'block-regression' } }); + expect(created.ok()).toBe(true); + const { contentId } = await created.json(); + const { plugin } = await (await page.request.get('/test/routes')).json(); + // A direct unsupported request is a controlled 400, never a type-mismatch exception. + const direct = await page.request.get(`${plugin}/PageUrl?contentId=${encodeURIComponent(contentId)}&locale=en`); + expect(direct.status()).toBe(400); + const blockUrlRequests: string[] = []; + page.on('request', request => { + const url = new URL(request.url()); + if (/\/PageUrl$/i.test(url.pathname) && url.searchParams.get('contentId')?.split('_')[0] === contentId.split('_')[0]) + blockUrlRequests.push('block page URL request'); + }); + await page.goto(`/episerver/cms/#context=epi.cms.contentdata:///${contentId}`); + await expect(page.getByText('Regression block', { exact: true }).filter({ visible: true }).first()).toBeVisible(); + await selectPage(page, 'Second page'); + await expect(page.locator('#overlay-context')).toContainText('/second-page'); + expect(blockUrlRequests).toEqual([]); + expect(evidence).toEqual([]); + expect(await snapshot()).toEqual(before); +});