diff --git a/.github/workflows/wpt-test.yml b/.github/workflows/wpt-test.yml index 27a3836..c2bb596 100644 --- a/.github/workflows/wpt-test.yml +++ b/.github/workflows/wpt-test.yml @@ -35,22 +35,6 @@ jobs: WPT_API_KEY: ${{ secrets.HA_API_KEY }} PR_BODY: ${{ github.event.pull_request.body }} - - name: Upload artifact if needed - id: artifact-upload-step - uses: actions/upload-artifact@v4 - with: - path: artifact.md - - - name: Update comment.md - run: | - if grep -q "{artifact-url}" comment.md; then - artifact_url="${{ steps.artifact-upload-step.outputs.artifact-url }}" - sed -i "s|{artifact-url}|$artifact_url|g" comment.md - echo "Placeholder {artifact-url} replaced with a value: $artifact_url." - else - echo "No placeholder {artifact-url} found in comment.md." - fi - - name: Add comment to PR uses: mshick/add-pr-comment@v2 if: always() diff --git a/dist/css-variables.js b/dist/css-variables.js index 16dfa84..bfaede9 100644 --- a/dist/css-variables.js +++ b/dist/css-variables.js @@ -1,286 +1,290 @@ //[css-variables] function analyzeVariables() { -const PREFIX = "almanac-var2020-"; + const PREFIX = "almanac-var2020-"; -// Selector to find elements that are relevant to the graph -const selector = `.${PREFIX}element, [style*="--"]`; + // Selector to find elements that are relevant to the graph + const selector = `.${PREFIX}element, [style*="--"]`; -// Extract a list of custom properties set by a value -function extractValueProperties(value) { - // https://drafts.csswg.org/css-syntax-3/#ident-token-diagram - let ret = value.match(/var\(--[-\w\u{0080}-\u{FFFF}]+(?=[,)])/gui)?.map(p => p.slice(4)); + // Extract a list of custom properties set by a value + function extractValueProperties(value) { + // https://drafts.csswg.org/css-syntax-3/#ident-token-diagram + let ret = value.match(/var\(--[-\w\u{0080}-\u{FFFF}]+(?=[,)])/gui)?.map(p => p.slice(4)); - if (ret) { - // Drop duplicates - return [...new Set(ret)]; + if (ret) { + // Drop duplicates + return [...new Set(ret)]; + } } -} -let visited = new Set(); -let modifiedRules = []; + let visited = new Set(); + let modifiedRules = []; -// Recursively walk a CSSStyleRule or CSSStyleDeclaration -function walkRule(rule, ret) { - if (!rule || visited.has(rule)) { - return; - } - - visited.add(rule); + // Recursively walk a CSSStyleRule or CSSStyleDeclaration + function walkRule(rule, ret) { + if (!rule || visited.has(rule)) { + return; + } - let style, selector; + visited.add(rule); - if (rule instanceof CSSStyleRule && rule.style) { - style = rule.style; - selector = rule.selectorText; - } - else if (rule instanceof CSSStyleDeclaration) { - style = rule; - selector = ""; - } + let style, selector; - if (style) { - let condition; - // mirror properties to add. We add them afterwards, so we don't pointlessly traverse them - let additions = {}; - for (let property of style) { - let value = style.getPropertyValue(property); + if (rule instanceof CSSStyleRule && rule.style) { + style = rule.style; + selector = rule.selectorText; + } + else if (rule instanceof CSSStyleDeclaration) { + style = rule; + selector = ""; + } - let containsRef = value.indexOf("var(--") > -1; - let setsVar = property.indexOf("--") === 0 && property.indexOf("--" + PREFIX) === -1; + if (style) { + let condition; + // mirror properties to add. We add them afterwards, so we don't pointlessly traverse them + let additions = {}; - if (containsRef || setsVar) { - if (!condition && rule.parentRule) { - condition = []; - let r = rule; + for (let property of style) { + let value = style.getPropertyValue(property); + if (value.startsWith("url(\"data:image/svg+xml") && value.length > 54) { + value = value.slice(0, 50) + '…' + "\");"; + } - while (r.parentRule?.conditionText) { - r = r.parentRule; - condition.push({ - type: r instanceof CSSMediaRule? "media" : "supports", - test: r.conditionText - }); + let containsRef = value.indexOf("var(--") > -1; + let setsVar = property.indexOf("--") === 0 && property.indexOf("--" + PREFIX) === -1; + + if (containsRef || setsVar) { + if (!condition && rule.parentRule) { + condition = []; + let r = rule; + + while (r.parentRule?.conditionText) { + r = r.parentRule; + condition.push({ + type: r instanceof CSSMediaRule ? "media" : "supports", + test: r.conditionText + }); + } } - } - if (containsRef) { - // Set mirror property so we can find it in the computed style - additions["--" + PREFIX + property] = value.replace(/var\(--/g, PREFIX + "$&"); + if (containsRef) { + // Set mirror property so we can find it in the computed style + additions["--" + PREFIX + property] = value.replace(/var\(--/g, PREFIX + "$&"); - let properties = extractValueProperties(value); + let properties = extractValueProperties(value); - for (let prop of properties) { - let info = ret[prop] = ret[prop] || {get: [], set: []}; - info.get.push({ usedIn: property, value, selector, condition }); + for (let prop of properties) { + let info = ret[prop] = ret[prop] || { get: [], set: [] }; + info.get.push({ usedIn: property, value, selector, condition }); + } } - } - if (setsVar) { - let info = ret[property] = ret[property] || {get: [], set: []}; - info.set.push({ value, selector, condition }); - } + if (setsVar) { + let info = ret[property] = ret[property] || { get: [], set: [] }; + info.set.push({ value, selector, condition }); + } - // Add class so we can find these later - if (selector) { - for (let el of document.querySelectorAll(selector)) { - el.classList.add(`${PREFIX}element`); + // Add class so we can find these later + if (selector) { + for (let el of document.querySelectorAll(selector)) { + el.classList.add(`${PREFIX}element`); + } } } } - } - // Now that we're done, add the mirror properties - for (let property in additions) { - modifiedRules.push({style, additions}); - style.setProperty(property, additions[property]); + // Now that we're done, add the mirror properties + for (let property in additions) { + modifiedRules.push({ style, additions }); + style.setProperty(property, additions[property]); + } } - } - if (rule instanceof CSSMediaRule || rule instanceof CSSSupportsRule) { - // rules with child rules, e.g. @media, @supports - for (let r of rule.cssRules) { - walkRule(r, ret); + if (rule instanceof CSSMediaRule || rule instanceof CSSSupportsRule) { + // rules with child rules, e.g. @media, @supports + for (let r of rule.cssRules) { + walkRule(r, ret); + } } } -} -// Return a subset of the DOM tree that contains variable reads or writes -function buildGraph() { - // Elements that contain variable reads or writes. - let elements = new Set(document.querySelectorAll(selector)); - let map = new Map(); // keep pointers to object for each element - let ret = []; + // Return a subset of the DOM tree that contains variable reads or writes + function buildGraph() { + // Elements that contain variable reads or writes. + let elements = new Set(document.querySelectorAll(selector)); + let map = new Map(); // keep pointers to object for each element + let ret = []; - for (let element of elements) { - map.set(element, {element, children: []}); - } + for (let element of elements) { + map.set(element, { element, children: [] }); + } - for (let element of elements) { - let ancestor = element.parentNode.closest?.(selector); - let obj = map.get(element); + for (let element of elements) { + let ancestor = element.parentNode.closest?.(selector); + let obj = map.get(element); - if (ancestor) { - let o = map.get(ancestor); - o.children.push(obj) - } - else { - // Top-level - ret.push(obj); - } + if (ancestor) { + let o = map.get(ancestor); + o.children.push(obj) + } + else { + // Top-level + ret.push(obj); + } - let cs = element.computedStyleMap(); - let parentCS = element.parentNode.computedStyleMap?.(); - let vars = extractVars(cs, parentCS); + let cs = element.computedStyleMap(); + let parentCS = element.parentNode.computedStyleMap?.(); + let vars = extractVars(cs, parentCS); - if (Object.keys(vars).length > 0) { - obj.declarations = vars; + if (Object.keys(vars).length > 0) { + obj.declarations = vars; + } } - } - return ret; -} + return ret; + } -// Extract custom property declarations from a computed style map -// The schema of the returned object is: -// {get: {--var1: [{property, value, computedValue}]}, set: {--var2: {value, type}}} -function extractVars(cs, parentCS) { - let ret = {}; - let norefs = {}; - - for (let [property, [originalValue]] of cs) { - // Do references first - if (property.indexOf("--") === 0) { - let value = originalValue + ""; - - // Skip inherited values - if (parentCS && (parentCS.get(property) + "" === value + "")) { - continue; // most likely inherited - } + // Extract custom property declarations from a computed style map + // The schema of the returned object is: + // {get: {--var1: [{property, value, computedValue}]}, set: {--var2: {value, type}}} + function extractVars(cs, parentCS) { + let ret = {}; + let norefs = {}; + + for (let [property, [originalValue]] of cs) { + // Do references first + if (property.indexOf("--") === 0) { + let value = originalValue + ""; + + // Skip inherited values + if (parentCS && (parentCS.get(property) + "" === value + "")) { + continue; // most likely inherited + } - if (property.indexOf("--" + PREFIX) === 0) { - // Usage - let originalProperty = property.replace("--" + PREFIX, ""); + if (property.indexOf("--" + PREFIX) === 0) { + // Usage + let originalProperty = property.replace("--" + PREFIX, ""); - value = value.replace(RegExp(PREFIX + "var\\(--", "g"), "var(--"); - let properties = extractValueProperties(value); - let computed = cs.get(originalProperty) + ""; + value = value.replace(RegExp(PREFIX + "var\\(--", "g"), "var(--"); + let properties = extractValueProperties(value); + let computed = cs.get(originalProperty) + ""; - ret[originalProperty] = { - value, - references: properties - } + ret[originalProperty] = { + value, + references: properties + } - if (computed !== value) { - ret[originalProperty].computed = computed; + if (computed !== value) { + ret[originalProperty].computed = computed; + } } - } - else { - // Definition - norefs[property] = {value}; + else { + // Definition + norefs[property] = { value }; - if (originalValue + "" !== value) { - norefs[property].computed = originalValue + ""; - } + if (originalValue + "" !== value) { + norefs[property].computed = originalValue + ""; + } - // If value is of another type, we have Houdini P&V usage! - if (!(originalValue instanceof CSSUnparsedValue)) { - norefs[property].type = Object.prototype.toString.call(originalValue).slice(8, -1); + // If value is of another type, we have Houdini P&V usage! + if (!(originalValue instanceof CSSUnparsedValue)) { + norefs[property].type = Object.prototype.toString.call(originalValue).slice(8, -1); + } } } } - } - // Merge static with ret - for (let property in norefs) { - if (!(property in ret)) { - ret[property] = norefs[property]; + // Merge static with ret + for (let property in norefs) { + if (!(property in ret)) { + ret[property] = norefs[property]; + } } - } - return ret; -} + return ret; + } -let summary = {}; + let summary = {}; -// Walk through stylesheet and add custom properties for every declaration that uses var() -// This way we can retrieve them in the computed styles and build a dependency graph. -// Otherwise, they get resolved before they hit the computed style. -for (let stylesheet of document.styleSheets) { - try { - var rules = stylesheet.cssRules; - } - catch (e) {} + // Walk through stylesheet and add custom properties for every declaration that uses var() + // This way we can retrieve them in the computed styles and build a dependency graph. + // Otherwise, they get resolved before they hit the computed style. + for (let stylesheet of document.styleSheets) { + try { + var rules = stylesheet.cssRules; + } + catch (e) { } - if (rules) { - for (let rule of rules) { - walkRule(rule, summary); + if (rules) { + for (let rule of rules) { + walkRule(rule, summary); + } } } -} -function collapseDuplicateSiblings(arr) { - if (arr) { - let map = {}; + function collapseDuplicateSiblings(arr) { + if (arr) { + let map = {}; - for (let child of arr) { - let serialized = serialize(child); + for (let child of arr) { + let serialized = serialize(child); - if (serialized in map) { - // Dupe - map[serialized]++; - } - else { - map[serialized] = 0; + if (serialized in map) { + // Dupe + map[serialized]++; + } + else { + map[serialized] = 0; + } } - } - let entries = Object.entries(map); + let entries = Object.entries(map); - if (entries.length < arr.length) { - // There are duplicates - arr = entries.map(e => { - let child = JSON.parse(e[0]); + if (entries.length < arr.length) { + // There are duplicates + arr = entries.map(e => { + let child = JSON.parse(e[0]); - if (e[1] > 0) { - child.times = e[1] + 1; - } + if (e[1] > 0) { + child.times = e[1] + 1; + } + + return child; + }) + } - return child; - }) + arr.forEach(o => { + o.children = collapseDuplicateSiblings(o.children); + }); } - arr.forEach(o => { - o.children = collapseDuplicateSiblings(o.children); - }); + return arr; } - return arr; -} - -// Do the same thing with inline styles -for (let element of document.querySelectorAll('[style*="--"]')) { - walkRule(element.style, summary); -} + // Do the same thing with inline styles + for (let element of document.querySelectorAll('[style*="--"]')) { + walkRule(element.style, summary); + } -let computed = buildGraph(); + let computed = buildGraph(); -// Cleanup: Remove classes -for (let el of document.querySelectorAll(`.${PREFIX}element`)) { - el.classList.remove(`${PREFIX}element`); -} + // Cleanup: Remove classes + for (let el of document.querySelectorAll(`.${PREFIX}element`)) { + el.classList.remove(`${PREFIX}element`); + } -// Cleanup: Remove custom properties -for (let o of modifiedRules) { - for (let prop in o.additions) { - o.style.removeProperty(prop); + // Cleanup: Remove custom properties + for (let o of modifiedRules) { + for (let prop in o.additions) { + o.style.removeProperty(prop); + } } -} -computed = collapseDuplicateSiblings(computed); + computed = collapseDuplicateSiblings(computed); -return {summary, computed}; + return { summary, computed }; }; diff --git a/tests/wpt.js b/tests/wpt.js index 4ae2d1f..f490b03 100644 --- a/tests/wpt.js +++ b/tests/wpt.js @@ -11,7 +11,7 @@ class WPTTestRunner { const wptApiKey = process.env.WPT_API_KEY; this.wpt = new WebPageTest(this.wptServer, wptApiKey); this.testResultsFile = 'comment.md'; - this.uploadArtifact = false; + this.commentSizeLimitHit = false; } /** @@ -40,21 +40,15 @@ class WPTTestRunner { * Check if the test results are too big for a comment * @param {number} stringLength Length of the test results string */ - checkCommentSize(stringLength) { - let commentSize = 0; - try { - commentSize = fs.statSync(testResultsFile).size; - } catch (err) { } - - if (commentSize + stringLength > 65536) { - const artifactFile = 'artifact.md'; - this.uploadArtifact = true; - if (commentSize > 0) { - fs.renameSync(this.testResultsFile, artifactFile); - } - fs.appendFileSync(this.testResultsFile, `Webpage test results that are too big for a comment are available as [the action's artifact]({artifact-url}).`); - this.testResultsFile = artifactFile; + checkCommentSizeLimit(stringLength) { + let commentSize = fs.statSync('comment.md', { throwIfNoEntry: false }) ? + fs.statSync('comment.md', {throwIfNoEntry: false})?.size : 0; + + if (commentSize + stringLength > 64500) { + this.commentSizeLimitHit = true; } + + return this.commentSizeLimitHit; } /** @@ -75,7 +69,6 @@ class WPTTestRunner { }); } - /** * Run a WebPageTest test for a given URL * @param {string} url URL to test @@ -102,19 +95,25 @@ class WPTTestRunner { const metricsToLogObject = this.extractLogMetrics(metricsObject, metricsToLog); const metricsToLogString = JSON.stringify(metricsToLogObject, null, 2); - if (!this.uploadArtifact) { - this.checkCommentSize(metricsToLogString.length); - } - - fs.appendFileSync(this.testResultsFile, `
-Custom metrics for ${url} + let WPTResults = `
${url} -WPT test run results: ${response.data.summary} +[WPT result details](https://webpagetest.httparchive.org/result/${response.data.id}/1/details/#result)\n`, + metricsObjectResult = ` Changed custom metrics values: \`\`\`json ${metricsToLogString} \`\`\` -
\n\n`); +
\n\n`, + metricsObjectJSON = ` +Cannot display changed custom metrics due to comment size limits, \ +[view test JSON](https://webpagetest.httparchive.org/jsonResult.php?test=${response.data.id}&pretty=1) instead. +\n\n`; + + if (!this.commentSizeLimitHit && !this.checkCommentSizeLimit(metricsToLogString.length)) { + fs.appendFileSync(this.testResultsFile, WPTResults + metricsObjectResult); + } else { + fs.appendFileSync(this.testResultsFile, WPTResults + metricsObjectJSON); + } console.log('::endgroup::'); return metricsObject;