From beeafd1fb0ab548ac167f507e39ffd7421493694 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Daoust?= Date: Mon, 26 Aug 2024 13:25:55 +0200 Subject: [PATCH] Merge missing task anomalies and add guidance (#675) This adds a generic mechanism to give additional guidance at the end of an issue file, typically useful to link to the relevant section in the HTML spec for missing "queue a global task" problems. Guidance may also be set at the anomaly group level if we ever need that. To avoid reporting the same guidance twice, I merged the two types of missing task anomalies. That also allows us to reuse the same `type+spec` structure as that used for broken links and discontinued references. (Minor side fix: the initial run of a CLI may take some time and `this.slow` in Mocha does not change the 2s timeout. Explicit call to `this.timeout` added.) --- src/lib/study-algorithms.js | 8 ++++---- src/lib/study.js | 24 ++++++++++++++---------- test/cli.js | 3 ++- test/study-algorithms.js | 4 ++-- test/study.js | 25 +++++++++++++++++++++++++ 5 files changed, 47 insertions(+), 17 deletions(-) diff --git a/src/lib/study-algorithms.js b/src/lib/study-algorithms.js index 2466a282..5f5be355 100644 --- a/src/lib/study-algorithms.js +++ b/src/lib/study-algorithms.js @@ -88,16 +88,16 @@ function studyAlgorithms(specs) { !html.includes('systemClipboardRepresentation') ) { report.push({ - name: 'missingTaskForPromise', - message: `${getAlgoName(algo)} has a parallel step that resolves/rejects a promise directly`, + name: 'missingTask', + message: `${getAlgoName(algo)} resolves/rejects a promise directly in a step that runs in parallel`, spec }); return true; } else if (html.match(/fire an?( \w+)? event/i)) { report.push({ - name: 'missingTaskForEvent', - message: `${getAlgoName(algo)} has a parallel step that fires an event directly`, + name: 'missingTask', + message: `${getAlgoName(algo)} fires an event directly in a step that runs in parallel`, spec }); return true; diff --git a/src/lib/study.js b/src/lib/study.js index 7f6264ea..850b003c 100644 --- a/src/lib/study.js +++ b/src/lib/study.js @@ -103,14 +103,10 @@ const anomalyGroups = [ description: 'The following problems were identified when analyzing algorithms', types: [ { - name: 'missingTaskForPromise', - title: 'Missing tasks in parallel steps to handle a promise', - description: 'The following algorithms resolve or reject a Promise within a step that runs [in parallel](https://html.spec.whatwg.org/multipage/infrastructure.html#in-parallel) without first queuing a task' - }, - { - name: 'missingTaskForEvent', - title: 'Missing tasks in parallel steps to fire an event', - description: 'The following algorithms fire an event within a step that runs [in parallel](https://html.spec.whatwg.org/multipage/infrastructure.html#in-parallel) without first queuing a task' + name: 'missingTask', + title: 'Missing tasks in parallel steps', + description: 'The following algorithms fire an event, or resolve or reject a Promise, within a step that runs [in parallel](https://html.spec.whatwg.org/multipage/infrastructure.html#in-parallel) without first queuing a task', + guidance: 'See [Dealing with the event loop](https://html.spec.whatwg.org/multipage/webappapis.html#event-loop-for-spec-authors) in the HTML specification for guidance on how to deal with algorithm sections that run *in parallel*.' } ], study: studyAlgorithms @@ -439,8 +435,16 @@ function serializeEntry(entry, format, depth = 0) { for (const item of entry.items ?? []) { res += '\n' + serializeEntry(item, format, depth + 1); } - for (const anomaly of entry.anomalies ?? []) { - res += `\n` + serializeEntry(anomaly, format, depth + 1); + if (entry.anomalies?.length > 0) { + for (const anomaly of entry.anomalies) { + res += `\n` + serializeEntry(anomaly, format, depth + 1); + } + if (entry.group?.guidance) { + res += `\n\n` + entry.group.guidance; + } + else if (entry.type?.guidance) { + res += `\n\n` + entry.type.guidance; + } } } return res; diff --git a/test/cli.js b/test/cli.js index 4c6ac976..2ba2ce24 100644 --- a/test/cli.js +++ b/test/cli.js @@ -23,6 +23,7 @@ async function strudy(params) { describe(`Strudy's CLI`, function () { this.slow(5000); + this.timeout(10000); it('reports usage help when asked', async function () { const { stdout, stderr } = await strudy(`--help`); @@ -71,6 +72,6 @@ describe(`Strudy's CLI`, function () { const { stdout, stderr } = await strudy(`inspect test/data/empty.json --issues issues --update-mode notamode`); assert.match(stderr, /Unsupported --update-mode option/); assert.deepEqual(stdout, ''); - }) + }); }); }); \ No newline at end of file diff --git a/test/study-algorithms.js b/test/study-algorithms.js index c5b19888..f12c52ff 100644 --- a/test/study-algorithms.js +++ b/test/study-algorithms.js @@ -29,8 +29,8 @@ describe('The algorithms analyser', () => { ]); const report = study(crawlResult); assertAnomaly(report, 0, { - name: 'missingTaskForPromise', - message: 'The algorithm that starts with "The encodingInfo() method MUST run the following steps:" has a parallel step that resolves/rejects a promise directly', + name: 'missingTask', + message: 'The algorithm that starts with "The encodingInfo() method MUST run the following steps:" resolves/rejects a promise directly in a step that runs in parallel', spec: { url: 'https://www.w3.org/TR/spec' } }); }); diff --git a/test/study.js b/test/study.js index 30c09386..d439997b 100644 --- a/test/study.js +++ b/test/study.js @@ -166,4 +166,29 @@ describe('The main study function', function () { const report = await study(crawlResult, { specs: ['universe'], htmlFragments: {} }); assertNbAnomalies(report.results, 1); }); + + it('reports guidance when possible', async function() { + const crawlResult = [ + populateSpec(specUrl, { + algorithms: [{ + html: 'The encodingInfo() method MUST run the following steps:', + rationale: 'if', + steps: [ + { html: 'Let p be a new promise.' }, + { html: 'In parallel, run the Create a MediaCapabilitiesEncodingInfo algorithm with configuration and resolve p with its result.' }, + { html: 'Return p.' } + ] + }] + }) + ]; + const report = await study(crawlResult, { htmlFragments: {} }); + assertNbAnomalies(report.results, 1); + assertAnomaly(report.results, 0, { + title: 'Missing tasks in parallel steps in Hello world API', + content: `While crawling [Hello world API](https://w3c.github.io/world/), the following algorithms fire an event, or resolve or reject a Promise, within a step that runs [in parallel](https://html.spec.whatwg.org/multipage/infrastructure.html#in-parallel) without first queuing a task: +* [ ] The algorithm that starts with \"The encodingInfo() method MUST run the following steps:\" resolves/rejects a promise directly in a step that runs in parallel + +See [Dealing with the event loop](https://html.spec.whatwg.org/multipage/webappapis.html#event-loop-for-spec-authors) in the HTML specification for guidance on how to deal with algorithm sections that run *in parallel*.` + }); + }); }); \ No newline at end of file