From 653205c117aab844e47faf2a3e93953d138bff21 Mon Sep 17 00:00:00 2001 From: vickydotbat Date: Sat, 20 Jun 2026 10:33:00 +0000 Subject: [PATCH] clean brittle tests (#3) Reviewed-on: https://git.westgate.pw/ShadowsOverWestgate/sow-nodebb-theme/pulls/3 Reviewed-by: xtul Co-authored-by: vickydotbat Co-committed-by: vickydotbat --- AGENTS.md | 4 + tests/post-vote-icons.test.js | 8 +- tests/wiki-breadcrumb-parity.test.js | 72 -------- tests/wiki-callout-icon-theme.test.js | 45 ----- tests/wiki-infobox-heading-layer.test.js | 53 ------ tests/wiki-mobile-table-client.test.js | 209 +++++++++++----------- tests/wiki-mobile-table-rendering.test.js | 84 ++++----- tests/wiki-tiptap-stack-order.test.js | 72 -------- 8 files changed, 142 insertions(+), 405 deletions(-) delete mode 100644 tests/wiki-breadcrumb-parity.test.js delete mode 100644 tests/wiki-callout-icon-theme.test.js delete mode 100644 tests/wiki-infobox-heading-layer.test.js delete mode 100644 tests/wiki-tiptap-stack-order.test.js diff --git a/AGENTS.md b/AGENTS.md index 3047280..16fbaa5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -54,3 +54,7 @@ NodeBB theme documentation: https://docs.nodebb.org/development/themes/ - Avoid unrelated formatting churn. - Before overriding a template, inspect the corresponding Harmony template and copy only what is necessary. - Preserve accessibility: readable contrast, visible focus states, usable hover/active states, and no text hidden purely for visual effect unless there is an accessible alternative. + +## Tests + +Tests must survive harmless changes to constants, defaults, wording, ordering, fixture data, and internal implementation details. A test that fails merely because a basic value changed is usually a bad test. Only assert exact values when the value is part of a documented public contract, external protocol, compatibility requirement, security rule, migration, or business rule. diff --git a/tests/post-vote-icons.test.js b/tests/post-vote-icons.test.js index 49e79f7..890df79 100644 --- a/tests/post-vote-icons.test.js +++ b/tests/post-vote-icons.test.js @@ -14,14 +14,14 @@ assert( assert.match( stylesheet, - /i\.fa-chevron-up\s*\{[^}]*--fa:\s*"\\f004"/s, - "The upvote button should reuse Harmony's Font Awesome element with the fa-heart glyph" + /i\.fa-chevron-up\s*\{[^}]*--fa:\s*"\\f[0-9a-f]+"/s, + "The upvote button should reuse Harmony's Font Awesome element" ); assert.match( stylesheet, - /i\.fa-chevron-down\s*\{[^}]*--fa:\s*"\\f7a9"/s, - "The downvote button should reuse Harmony's Font Awesome element with the fa-heart-crack glyph" + /i\.fa-chevron-down\s*\{[^}]*--fa:\s*"\\f[0-9a-f]+"/s, + "The downvote button should reuse Harmony's Font Awesome element" ); assert.doesNotMatch( diff --git a/tests/wiki-breadcrumb-parity.test.js b/tests/wiki-breadcrumb-parity.test.js deleted file mode 100644 index f64962d..0000000 --- a/tests/wiki-breadcrumb-parity.test.js +++ /dev/null @@ -1,72 +0,0 @@ -const assert = require("assert"); -const fs = require("fs"); -const path = require("path"); - -const stylesheet = fs.readFileSync( - path.join(__dirname, "..", "scss", "westgate", "_wiki-prose.scss"), - "utf8" -); - -function ruleHasDeclaration(selector, declaration) { - const rulePattern = /([^{}]+)\{([^{}]*)\}/g; - let match; - while ((match = rulePattern.exec(stylesheet)) !== null) { - const selectors = match[1].split(",").map(value => value.trim()); - const body = match[2]; - if (selectors.includes(selector) && body.includes(declaration)) { - return true; - } - } - return false; -} - -assert( - ruleHasDeclaration(".westgate-wiki .wiki-breadcrumb-trail__action", "padding-left: 12px"), - "Wiki edit action should keep the same left-side separator spacing as breadcrumb items" -); - -[ - "--wiki-breadcrumb-color: color-mix(in srgb, var(--wg-text-muted) 68%, transparent)", - "font-family: var(--wg-font-display)", - "font-size: 0.875rem", - "gap: 0", - "font-weight: 600", - "letter-spacing: 0", - "text-transform: none", -].forEach(declaration => { - assert( - ruleHasDeclaration(".westgate-wiki .wiki-breadcrumb-trail", declaration), - `Wiki breadcrumbs should inherit regular forum breadcrumb typography: ${declaration}` - ); -}); - -assert( - ruleHasDeclaration(".westgate-wiki .wiki-breadcrumb-trail__list", "gap: 0"), - "Wiki breadcrumbs should use Bootstrap breadcrumb spacing instead of a custom flex gap" -); - -assert( - ruleHasDeclaration(".westgate-wiki .wiki-breadcrumb-trail__item:not(:last-child)::after", "content: none"), - "Wiki breadcrumbs should suppress the plugin slash divider" -); - -assert( - ruleHasDeclaration(".westgate-wiki .wiki-breadcrumb-trail__item + .wiki-breadcrumb-trail__item::before", "content: \"→\""), - "Wiki breadcrumbs should use the same arrow divider as regular forum breadcrumbs" -); - -[ - ".westgate-wiki .wiki-breadcrumb-trail__link", - ".westgate-wiki .wiki-breadcrumb-trail__text", - ".westgate-wiki .wiki-breadcrumb-trail__text--current", - ".westgate-wiki .wiki-breadcrumb-trail__action", -].forEach(selector => { - assert( - ruleHasDeclaration(selector, "color: var(--wiki-breadcrumb-color)"), - `${selector} should use the dimmed forum breadcrumb color` - ); - assert( - ruleHasDeclaration(selector, "font-family: var(--wg-font-display)"), - `${selector} should explicitly use the forum breadcrumb display font` - ); -}); diff --git a/tests/wiki-callout-icon-theme.test.js b/tests/wiki-callout-icon-theme.test.js deleted file mode 100644 index 9ce8722..0000000 --- a/tests/wiki-callout-icon-theme.test.js +++ /dev/null @@ -1,45 +0,0 @@ -const assert = require("assert"); -const fs = require("fs"); -const path = require("path"); - -const stylesheet = fs.readFileSync( - path.join(__dirname, "..", "scss", "westgate", "_wiki-prose.scss"), - "utf8" -); - -function ruleHasDeclaration(selector, declaration) { - const rulePattern = /([^{}]+)\{([^{}]*)\}/g; - let match; - while ((match = rulePattern.exec(stylesheet)) !== null) { - const selectors = match[1].split(",").map(value => value.trim()); - const body = match[2]; - if (selectors.includes(selector) && body.includes(declaration)) { - return true; - } - } - return false; -} - -[ - ".westgate-wiki .wiki-article-prose .wiki-callout::after", - ".westgate-wiki-compose .wiki-editor__content .wiki-callout::after", -].forEach(selector => { - assert( - ruleHasDeclaration(selector, "mask-mode: alpha"), - `${selector} should force alpha mask rendering without changing the plugin's icon color` - ); - assert( - ruleHasDeclaration(selector, "-webkit-mask-mode: alpha"), - `${selector} should force alpha mask rendering on WebKit/Blink mobile browsers` - ); -}); - -assert( - !stylesheet.includes("--wiki-callout-icon-color"), - "The theme should not introduce new callout icon colors for this mobile rendering fix" -); - -assert( - !stylesheet.includes(".wiki-callout::before"), - "The theme should not restyle the callout icon tile for this mobile rendering fix" -); diff --git a/tests/wiki-infobox-heading-layer.test.js b/tests/wiki-infobox-heading-layer.test.js deleted file mode 100644 index 53f5fa5..0000000 --- a/tests/wiki-infobox-heading-layer.test.js +++ /dev/null @@ -1,53 +0,0 @@ -const assert = require("assert"); -const fs = require("fs"); -const path = require("path"); - -const stylesheet = fs.readFileSync( - path.join(__dirname, "..", "scss", "westgate", "_wiki-prose.scss"), - "utf8" -); - -function ruleHasDeclaration(selector, declaration) { - const rulePattern = /([^{}]+)\{([^{}]*)\}/g; - let match; - while ((match = rulePattern.exec(stylesheet)) !== null) { - const selectors = match[1].split(",").map(value => value.trim()); - const body = match[2]; - if (selectors.includes(selector) && body.includes(declaration)) { - return true; - } - } - return false; -} - -function matchingRuleHasDeclaration(selectorPattern, declaration) { - const rulePattern = /([^{}]+)\{([^{}]*)\}/g; - let match; - while ((match = rulePattern.exec(stylesheet)) !== null) { - const selectors = match[1].split(",").map(value => value.trim()); - const body = match[2]; - if (selectors.some(selector => selectorPattern.test(selector)) && body.includes(declaration)) { - return true; - } - } - return false; -} - -[ - ".westgate-wiki .wiki-article-prose .wiki-infobox", - ".westgate-wiki-compose .wiki-editor__content .wiki-infobox", -].forEach(selector => { - assert( - ruleHasDeclaration(selector, "position: relative"), - `${selector} should establish a positioned layer above overlapping prose headings` - ); - assert( - ruleHasDeclaration(selector, "z-index: var(--wiki-infobox-layer, 1)"), - `${selector} should stay selectable when a positioned heading's full block box overlaps it` - ); -}); - -assert( - !matchingRuleHasDeclaration(/\bh[1-6]\b/, "pointer-events: none"), - "Infobox/heading selection should not be fixed by disabling pointer events on prose content" -); diff --git a/tests/wiki-mobile-table-client.test.js b/tests/wiki-mobile-table-client.test.js index ec9f704..cc470cf 100644 --- a/tests/wiki-mobile-table-client.test.js +++ b/tests/wiki-mobile-table-client.test.js @@ -9,20 +9,17 @@ const source = fs.readFileSync( path.join(__dirname, '..', 'public', 'client.js'), 'utf8' ); +const stylesheet = fs.readFileSync( + path.join(__dirname, '..', 'scss', 'westgate', '_wiki-prose.scss'), + 'utf8' +); const readyCallbacks = []; const eventHandlers = []; +const wrappers = []; const fakeApi = { get: function () {}, }; -const fakeDocument = { - querySelectorAll: function () { - return []; - }, - createElement: function () { - throw new Error('createElement should not be called during initialisation'); - }, -}; function fakeJquery() { return { @@ -35,12 +32,83 @@ function fakeJquery() { }; } +function createParent() { + return { + children: [], + insertBefore: function (node, before) { + const index = this.children.indexOf(before); + assert.notEqual(index, -1, 'table fixture should belong to its parent'); + this.children.splice(index, 0, node); + node.parentNode = this; + }, + }; +} + +function createTable(parent, options = {}) { + const cells = [{ marker: 'first' }, { marker: 'second' }]; + const table = { + parentNode: parent, + style: {}, + rows: [{ children: cells }], + closest: function () { + return options.existingWrapper || null; + }, + getAttribute: function () { + return null; + }, + getBoundingClientRect: function () { + return { width: 320 }; + }, + querySelectorAll: function (selector) { + if (selector === 'tr') { + return this.rows; + } + return []; + }, + }; + parent.children.push(table); + return table; +} + +function createWrapper() { + const wrapper = { + attributes: {}, + children: [], + style: { + setProperty: function () {}, + }, + appendChild: function (child) { + const oldParent = child.parentNode; + const oldIndex = oldParent.children.indexOf(child); + oldParent.children.splice(oldIndex, 1); + this.children.push(child); + child.parentNode = this; + }, + setAttribute: function (name, value) { + this.attributes[name] = value; + }, + }; + wrappers.push(wrapper); + return wrapper; +} + +const firstParent = createParent(); +const firstTable = createTable(firstParent); +let queriedTables = [firstTable]; +const fakeDocument = { + querySelectorAll: function () { + return queriedTables; + }, + createElement: function () { + return createWrapper(); + }, +}; + const context = { console, document: fakeDocument, require: function (deps, callback) { - assert.equal(deps.length, 1); - assert.equal(deps[0], 'api'); + assert(Array.isArray(deps)); callback(fakeApi); }, window: {}, @@ -50,104 +118,33 @@ const context = { vm.runInNewContext(source, context, { filename: 'public/client.js' }); readyCallbacks.forEach(callback => callback()); -assert.equal( - typeof (context.window.westgateTheme && context.window.westgateTheme.wrapWikiTables), - 'function', - 'client should expose a testable wiki table wrapper helper' -); - +assert.equal(wrappers.length, 1, 'plain rendered wiki tables should gain a scroll container'); +assert.equal(wrappers[0].children[0], firstTable, 'wrapping must preserve the original table node'); assert( - eventHandlers.some(handler => handler.eventName === 'action:ajaxify.end'), - 'client should rerun wiki table wrapping after ajaxify navigation' + wrappers[0].className && stylesheet.includes(`.${wrappers[0].className}`), + 'the generated scroll container should be handled by the theme stylesheet' ); +assert.deepEqual( + firstTable.rows[0].children.map(cell => cell.marker), + ['first', 'second'], + 'wrapping must preserve table cell order and semantics' +); +assert.equal(wrappers[0].attributes.tabindex, '0', 'the scroll container should be keyboard focusable'); +assert(wrappers[0].attributes['aria-label'], 'the scroll container should have an accessible name'); -let createdWrapper; -let insertedNode; -const parent = { - insertBefore: function (node, before) { - insertedNode = { node, before }; - node.parentNode = this; - }, -}; -const table = { - style: {}, - parentNode: parent, - closest: function (selector) { - assert( - selector.includes('wg-mobile-table-scroll') || - selector.includes('tableWrapper') || - selector.includes('wiki-infobox'), - `unexpected closest selector: ${selector}` - ); - return null; - }, - getAttribute: function (name) { - return name === 'style' ? 'width:100%' : null; - }, - getBoundingClientRect: function () { - return { width: 320 }; - }, - querySelectorAll: function (selector) { - if (selector === 'colgroup col') { - return [ - { style: { width: '61px' }, getAttribute: function () { return 'width:61px'; } }, - { style: { width: '80px' }, getAttribute: function () { return 'width:80px'; } }, - ]; - } +const ajaxifyHandler = eventHandlers.find(handler => handler.eventName === 'action:ajaxify.end'); +assert(ajaxifyHandler, 'client should rerun wiki table wrapping after NodeBB ajaxify navigation'); - if (selector === 'tr') { - return [ - { - children: Array.from({ length: 10 }, () => ({ - getAttribute: function () { - return null; - }, - })), - }, - ]; - } +const secondParent = createParent(); +const secondTable = createTable(secondParent); +queriedTables = [secondTable]; +ajaxifyHandler.callback(); +assert.equal(wrappers.length, 2, 'tables loaded by ajaxify navigation should also be wrapped'); +assert.equal(wrappers[1].children[0], secondTable); - return []; - }, -}; -const root = { - querySelectorAll: function (selector) { - assert.equal( - selector, - '.westgate-wiki .wiki-page-content.wiki-article-prose > .card-body table' - ); - return [table]; - }, -}; - -context.document.createElement = function (tagName) { - assert.equal(tagName, 'div'); - createdWrapper = { - className: '', - child: null, - attributes: {}, - style: { - values: {}, - setProperty: function (name, value) { - this.values[name] = value; - }, - }, - appendChild: function (child) { - this.child = child; - child.parentNode = this; - }, - setAttribute: function (name, value) { - this.attributes[name] = value; - }, - }; - return createdWrapper; -}; - -context.window.westgateTheme.wrapWikiTables(root); - -assert.equal(createdWrapper.className, 'wg-mobile-table-scroll'); -assert.equal(createdWrapper.attributes.tabindex, '0'); -assert.equal(insertedNode.node, createdWrapper); -assert.equal(insertedNode.before, table); -assert.equal(createdWrapper.child, table); -assert.equal(createdWrapper.style.values['--wg-mobile-table-min-width'], '720px'); +const existingWrapper = {}; +const wrappedParent = createParent(); +const alreadyWrappedTable = createTable(wrappedParent, { existingWrapper }); +queriedTables = [alreadyWrappedTable]; +ajaxifyHandler.callback(); +assert.equal(wrappers.length, 2, 'tables already handled by a compatible wrapper should not be nested again'); diff --git a/tests/wiki-mobile-table-rendering.test.js b/tests/wiki-mobile-table-rendering.test.js index 82c5ee4..f703691 100644 --- a/tests/wiki-mobile-table-rendering.test.js +++ b/tests/wiki-mobile-table-rendering.test.js @@ -7,70 +7,48 @@ const stylesheet = fs.readFileSync( "utf8" ); -const mobileMediaMatch = stylesheet.match(/@media\s*\(max-width:\s*767\.98px\)\s*\{[\s\S]*\n\}/); -assert(mobileMediaMatch, "Expected a mobile-width table rendering media query"); +function readBlock(source, openingPattern) { + const opening = openingPattern.exec(source); + assert(opening, "Expected a mobile-width rendering media query"); -const mobileStyles = mobileMediaMatch[0]; -const infoboxWidth = "calc(100% - var(--wiki-infobox-reader-width) - var(--wiki-infobox-reader-gutter))"; -const infoboxWidthPattern = infoboxWidth.replace(/[()]/g, "\\$&").replace(/\s+/g, "\\s*"); + const blockStart = opening.index + opening[0].length; + let depth = 1; + for (let index = blockStart; index < source.length; index += 1) { + if (source[index] === "{") { + depth += 1; + } else if (source[index] === "}") { + depth -= 1; + if (depth === 0) { + return source.slice(blockStart, index); + } + } + } -[ - ".westgate-wiki .wiki-article-prose .wg-mobile-table-scroll", - ".westgate-wiki .wiki-article-prose .wg-mobile-table-scroll > table", - ".westgate-wiki .wiki-article-prose .wg-mobile-table-scroll > table.wiki-table-layout-fixed[style*=\"width:100%\"]", -].forEach(selector => { - assert( - mobileStyles.includes(selector), - `${selector} should receive mobile table handling without changing article HTML` - ); -}); + assert.fail("Mobile-width rendering media query is not closed"); +} -[ - "overflow-x: auto", - "-webkit-overflow-scrolling: touch", - "max-inline-size: 100%", - "max-inline-size: none", - "min-inline-size: var(--wg-mobile-table-min-width, max-content)", - "min-width: var(--wg-mobile-table-min-width, max-content)", -].forEach(declaration => { - assert( - mobileStyles.includes(declaration), - `Mobile wiki tables should include ${declaration}` - ); -}); +const mobileStyles = readBlock(stylesheet, /@media\s*\(max-width:[^)]+\)\s*\{/g); +assert.match( + mobileStyles, + /wiki[\s\S]*table[\s\S]*overflow-x:\s*auto/i, + "Wide wiki tables should scroll horizontally at mobile widths" +); [ "attr(data-label)", "wiki-mobile-table", - ".wiki-article-prose tr {\n\t\tdisplay: block", - ".wiki-article-prose td::before", - ".wiki-article-prose th::before", ].forEach(fragment => { assert( - !mobileStyles.includes(fragment), + !mobileStyles.includes(fragment), `Mobile table rendering must not use stacked/card conversion: ${fragment}` ); }); -assert( - !mobileStyles.includes(".westgate-wiki .wiki-page-content.wiki-article-prose > .card-body {\n\t\tmax-inline-size: 100%;\n\t\toverflow-x: auto"), - "Mobile table scrolling should be scoped to table wrappers, not the whole article body" -); - -assert.match( - stylesheet, - new RegExp( - "@media\\s*\\(min-width:\\s*768px\\)\\s*\\{[\\s\\S]*" + - "\\.westgate-wiki\\s+\\.wiki-article-prose\\s+\\.wiki-infobox\\s*~\\s*\\.wg-mobile-table-scroll\\s*\\{[^}]*" + - "width:\\s*" + infoboxWidthPattern + "[^}]*" + - "max-width:\\s*" + infoboxWidthPattern, - "s" - ), - "Desktop article tables wrapped by the theme's mobile scroll helper should still shrink beside floated infoboxes" -); - -assert.match( - stylesheet, - /@media\s*\(min-width:\s*768px\)\s*\{[\s\S]*\.westgate-wiki\s+\.wiki-article-prose\s+\.wiki-infobox\s*~\s*\.wg-mobile-table-scroll\s*>\s*table\[style\*="width:100%"\],\s*\.westgate-wiki\s+\.wiki-article-prose\s+\.wiki-infobox\s*~\s*\.wg-mobile-table-scroll\s*>\s*table\[style\*="width: 100%"\]\s*\{[^}]*width:\s*100%\s*!important[^}]*max-width:\s*100%/s, - "Fluid-width tables inside the theme's scroll wrapper should fill the shrunken infobox-safe wrapper" +assert.doesNotMatch( + Array.from(mobileStyles.matchAll(/([^{}]+)\{([^{}]*)\}/g)) + .filter(([, selectors]) => /(^|[\s,>+~])(?:tr|td|th)(?=[:.#\[\s,>+~]|$)/i.test(selectors)) + .map(([, selectors, declarations]) => `${selectors}{${declarations}}`) + .join("\n"), + /display:\s*(?:block|grid)/i, + "Mobile rendering must preserve native table rows and cells" ); diff --git a/tests/wiki-tiptap-stack-order.test.js b/tests/wiki-tiptap-stack-order.test.js deleted file mode 100644 index ecedd2f..0000000 --- a/tests/wiki-tiptap-stack-order.test.js +++ /dev/null @@ -1,72 +0,0 @@ -const assert = require("assert"); -const fs = require("fs"); -const path = require("path"); - -const stylesheet = fs.readFileSync( - path.join(__dirname, "..", "scss", "westgate", "_wiki-prose.scss"), - "utf8" -); - -function readNumericCustomProperty(name) { - const pattern = new RegExp(`${name}:\\s*(\\d+)\\s*;`); - const match = stylesheet.match(pattern); - assert(match, `Expected ${name} to be defined as a numeric custom property`); - return Number(match[1]); -} - -function ruleHasDeclaration(selector, declaration) { - const rulePattern = /([^{}]+)\{([^{}]*)\}/g; - let match; - while ((match = rulePattern.exec(stylesheet)) !== null) { - const selectors = match[1].split(",").map(value => value.trim()); - const body = match[2]; - if (selectors.includes(selector) && body.includes(declaration)) { - return true; - } - } - return false; -} - -const tableHandleLayer = readNumericCustomProperty("--wiki-editor-table-handle-layer"); -const toolbarLayer = readNumericCustomProperty("--wiki-editor-toolbar-layer"); -const floatingToolbarLayer = readNumericCustomProperty("--wiki-editor-floating-toolbar-layer"); - -assert( - tableHandleLayer < floatingToolbarLayer, - "TipTap table resize handles must stack below content pop-out toolbars" -); -assert( - floatingToolbarLayer < toolbarLayer, - "Content pop-out toolbars must stack below the persistent main/table toolbar layer" -); - -[ - ".westgate-wiki-compose .wiki-editor__content .column-resize-handle", - ".westgate-wiki-compose .wiki-editor-table-resize-handle", -].forEach(selector => { - assert( - ruleHasDeclaration(selector, "z-index: var(--wiki-editor-table-handle-layer, 4)"), - `${selector} should use the table handle stack layer` - ); -}); - -[ - ".westgate-wiki-compose .wiki-editor__toolbar-mount", - ".westgate-wiki-compose .wiki-editor-table-sticky-row", -].forEach(selector => { - assert( - ruleHasDeclaration(selector, "z-index: var(--wiki-editor-toolbar-layer, 30)"), - `${selector} should use the toolbar stack layer` - ); -}); - -[ - ".westgate-wiki-compose .wiki-editor-context-tools", - ".westgate-wiki-compose .wiki-editor-image-tools", - ".westgate-wiki-compose .wiki-editor-table-cell-popover", -].forEach(selector => { - assert( - ruleHasDeclaration(selector, "z-index: var(--wiki-editor-floating-toolbar-layer, 20)"), - `${selector} should use the floating toolbar stack layer` - ); -});