Skip to content

Commit

Permalink
Merge missing task anomalies and add guidance (#675)
Browse files Browse the repository at this point in the history
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.)
  • Loading branch information
tidoust authored Aug 26, 2024
1 parent 7d9c18d commit beeafd1
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 17 deletions.
8 changes: 4 additions & 4 deletions src/lib/study-algorithms.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
24 changes: 14 additions & 10 deletions src/lib/study.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion test/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`);
Expand Down Expand Up @@ -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, '');
})
});
});
});
4 changes: 2 additions & 2 deletions test/study-algorithms.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' }
});
});
Expand Down
25 changes: 25 additions & 0 deletions test/study.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 <code class="idl"><a data-link-type="idl" href="https://w3c.github.io/media-capabilities/#dom-mediacapabilities-encodinginfo" id="ref-for-dom-mediacapabilities-encodinginfo">encodingInfo()</a></code> method MUST run the following steps:',
rationale: 'if',
steps: [
{ html: 'Let <var>p</var> be a new promise.' },
{ html: '<a data-link-type="dfn" href="https://html.spec.whatwg.org/multipage/infrastructure.html#in-parallel" id="ref-for-in-parallel①">In parallel</a>, run the <a data-link-type="dfn" href="https://w3c.github.io/media-capabilities/#create-a-mediacapabilitiesencodinginfo" id="ref-for-create-a-mediacapabilitiesencodinginfo">Create a MediaCapabilitiesEncodingInfo</a> algorithm with <var>configuration</var> and resolve <var>p</var> with its result.' },
{ html: 'Return <var>p</var>.' }
]
}]
})
];
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*.`
});
});
});

0 comments on commit beeafd1

Please sign in to comment.