From 91aaffe5335f95f01daf6f893f4e3a9814dbeb0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Daoust?= Date: Tue, 26 Mar 2024 19:10:40 +0100 Subject: [PATCH] Test monitor/ignore entries against main list, adjust find-specs (#1283) This addresses #238, adding checks that there are no entries in the monitor or ignore list that are already in the main list. This also adjusts the find-specs script to: 1. skip fragments in repository homepage URLs (`WICG/close-watcher` does that) 2. also look at spec pages to find URLs, to avoid reporting a spec that is already in the list (`WICG/close-watcher` also does that as it now points to an HTML page) 3. compare strings in a case insensitive way, as we sometimes end up with different casing for repo names and URLs. A couple of entries needed to be removed from the monitor/ignore list as a result ;) --- src/data/ignore.json | 6 ------ src/data/monitor.json | 4 ---- src/find-specs.js | 17 +++++++++++++-- test/data.js | 48 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 63 insertions(+), 12 deletions(-) diff --git a/src/data/ignore.json b/src/data/ignore.json index 342efd28..47af4d7f 100644 --- a/src/data/ignore.json +++ b/src/data/ignore.json @@ -80,9 +80,6 @@ "w3c/sparql-new": { "comment": "group note and not targeted at browsers" }, - "WICG/close-watcher": { - "comment": "merged into HTML" - }, "WICG/discourse-archive": { "comment": "not a spec" }, @@ -373,9 +370,6 @@ "https://tc39.es/proposal-regexp-legacy-features/": { "comment": "no proper spec, legacy" }, - "https://tc39.es/proposal-resizablearraybuffer/": { - "comment": "Integrated in ECMAScript" - }, "https://tc39.es/proposal-hashbang/": { "comment": "not meant for browsers environments" }, diff --git a/src/data/monitor.json b/src/data/monitor.json index 63f3aadf..48a869f5 100644 --- a/src/data/monitor.json +++ b/src/data/monitor.json @@ -144,10 +144,6 @@ "lastreviewed": "2024-03-01", "comment": "no published content yet" }, - "immersive-web/real-world-geometry": { - "lastreviewed": "2024-03-01", - "comment": "no published content yet" - }, "immersive-web/spatial-favicons": { "lastreviewed": "2024-03-01", "comment": "no published content yet" diff --git a/src/find-specs.js b/src/find-specs.js index f7bfa575..3a2aecfd 100644 --- a/src/find-specs.js +++ b/src/find-specs.js @@ -38,6 +38,13 @@ function canonicalizeGhUrl(r) { if (url.pathname.lastIndexOf('/') === 0 && url.pathname.length > 1) { url.pathname += '/'; } + + // Exceptionally, the homepage URL may link to a fragment within a spec. One + // example at the time of writing is Close Watcher to redirect to the HTML + // spec: https://github.com/WICG/close-watcher + if (url.hash) { + url.hash = ''; + } return {repo: r.owner.login + '/' + r.name, spec: url.toString()}; } @@ -71,10 +78,16 @@ const hasMoreRecentLevel = (s, url, loose) => { return false; } }; -const hasUntrackedURL = ({spec: url}) => !specs.find(s => s.nightly?.url.startsWith(trimSlash(url)) - || (s.release && trimSlash(s.release.url) === trimSlash(url))) +const hasUntrackedURL = ({spec: url}) => { + // Compare URLs case-insentively as we sometimes end up with different + // casing (and difference is usually not significant) + const lurl = trimSlash(url.toLowerCase()); + return !specs.find(s => s.nightly?.url?.toLowerCase()?.startsWith(lurl) + || (s.release && trimSlash(s.release.url.toLowerCase()) === lurl) + || (s.nightly?.pages && s.nightly.pages.find(u => trimSlash(u.toLowerCase()) === lurl))) && !specs.find(s => hasMoreRecentLevel(s, url, url.match(/\/drafts\./) && !url.match(/\/w3\.org/) // Because CSS specs have editors draft with and without levels, we look loosely for more recent levels when checking with editors draft )); +}; const hasUnknownTrSpec = ({spec: url}) => !specs.find(s => s.release && trimSlash(s.release.url) === trimSlash(url)) && !specs.find(s => hasMoreRecentLevel(s,url)); const eitherFilter = (f1, f2) => value => f1(value) || f2(value); diff --git a/test/data.js b/test/data.js index 3fd16046..03ba231b 100644 --- a/test/data.js +++ b/test/data.js @@ -25,6 +25,30 @@ describe("Ignore/Monitor lists", () => { const isValid = validate(list, { format: "full" }); assert.strictEqual(validate.errors, null); }); + + it("does not contain repos that are in the main list", () => { + const list = require("../src/data/ignore.json"); + const main = require("../index.json"); + const wrongRepos = Object.keys(list.repos).filter(repo => { + const githubRepo = `https://github.com/${repo}`.toLowerCase(); + return main.find(spec => + spec.nightly?.repository?.toLowerCase() === githubRepo); + }); + assert.deepStrictEqual(wrongRepos, []); + }); + + it("does not contain specs that are in the main list", () => { + const list = require("../src/data/ignore.json"); + const main = require("../index.json"); + const wrongSpecs = Object.keys(list.specs).filter(url => { + const lurl = url.toLowerCase(); + return main.find(spec => + spec.url.toLowerCase() === lurl || + spec.nightly?.url?.toLowerCase() === lurl || + spec.release?.url?.toLowerCase() === lurl); + }); + assert.deepStrictEqual(wrongSpecs, []); + }); }); describe("The monitor list", () => { @@ -47,6 +71,30 @@ describe("Ignore/Monitor lists", () => { .map(([key, value]) => key); assert.deepStrictEqual(wrongSpecs, []); }); + + it("does not contain repos that are in the main list", () => { + const list = require("../src/data/monitor.json"); + const main = require("../index.json"); + const wrongRepos = Object.keys(list.repos).filter(repo => { + const githubRepo = `https://github.com/${repo}`.toLowerCase(); + return main.find(spec => + spec.nightly?.repository?.toLowerCase() === githubRepo); + }); + assert.deepStrictEqual(wrongRepos, []); + }); + + it("does not contain specs that are in the main list", () => { + const list = require("../src/data/monitor.json"); + const main = require("../index.json"); + const wrongSpecs = Object.keys(list.specs).filter(url => { + const lurl = url.toLowerCase(); + return main.find(spec => + spec.url.toLowerCase() === lurl || + spec.nightly?.url?.toLowerCase() === lurl || + spec.release?.url?.toLowerCase() === lurl); + }); + assert.deepStrictEqual(wrongSpecs, []); + }); }); describe("An entry in one of the lists", () => {