fix(reporter): Run inside isolated contexts #3129
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
straker
previously requested changes
Aug 20, 2021
lib/core/reporters/no-passes.js
Outdated
|
|
||
| const noPassesReporter = (results, options, callback) => { | ||
| if (typeof options === 'function') { | ||
| callback = options; | ||
| options = {}; | ||
| } | ||
| const environmentData = options.environmentData; |
There was a problem hiding this comment.
Rather than delete on options we should just destructor it
const { environmentData, ...toolOptions } = options;
var out = processAggregate(results, toolOptions);
callback({
...getEnvironmentData(environmentData),
toolOptions,
violations: out.violations
});There was a problem hiding this comment.
... yeah that feels like a thing I should have thought of. Good one.
| }, | ||
| testEnvironment: getTestEnvironment(win), | ||
| timestamp: new Date().toISOString(), | ||
| url: win?.location?.href |
There was a problem hiding this comment.
We already know win is an object so no need to optionally chain it
Suggested change
| url: win?.location?.href | |
| url: win.location?.href |
| } | ||
|
|
||
| function getTestEnvironment(win) { | ||
| if (typeof window !== 'object' || typeof window.navigator !== 'object') { |
There was a problem hiding this comment.
Already know win is an object from the top-level call (also need to use win instead of window)
Suggested change
| if (typeof window !== 'object' || typeof window.navigator !== 'object') { | |
| if (typeof win.navigator !== 'object') { |
| const { navigator, innerHeight, innerWidth } = win; | ||
| const orientation = getOrientation(win); | ||
| return { | ||
| userAgent: navigator?.userAgent, |
There was a problem hiding this comment.
Already know win.navigator is an object so no need to optionally chain it
Suggested change
| userAgent: navigator?.userAgent, | |
| userAgent: navigator.userAgent, |
test/core/public/run-partial.js
Outdated
| @@ -129,6 +129,30 @@ describe('axe.runPartial', function() { | |||
| }); | |||
| }); | |||
|
|
|||
| describe('environmentData', function () { | |||
| it('is include environment data for the initiator', function (done) { | |||
There was a problem hiding this comment.
Suggested change
| it('is include environment data for the initiator', function (done) { | |
| it('includes environment data for the initiator', function (done) { |
| @@ -204,15 +204,27 @@ describe('reporters - na', function() { | |||
| }); | |||
| it('should add environment data', function() { | |||
| axe.getReporter('na')(runResults, {}, function(results) { | |||
| assert.isNotNull(results.url); | |||
| assert.isNotNull(results.timestamp); | |||
| assert.isNotNull(results.testEnvironement); | |||
| @@ -107,22 +107,22 @@ describe('reporters - no-passes', function() { | |||
| }); | |||
| }); | |||
| it('should add the rule id to the rule result', function() { | |||
| axe.getReporter('na')(runResults, {}, function(results) { | |||
| axe.getReporter('no-passes')(runResults, {}, function(results) { | |||
| parserOptions: { | ||
| sourceType: 'module' | ||
| }, | ||
| env: {}, |
There was a problem hiding this comment.
To avoid the exclusion above, just do:
Suggested change
| env: {}, | |
| env: { | |
| browser: false, | |
| etc: ... | |
| }, |
straker
approved these changes
Aug 23, 2021
straker
requested changes
Aug 23, 2021
stephenmathieson
previously requested changes
Aug 23, 2021
|
@stephenmathieson Yes, but one override doesn't cascade to another. |
Not worth requesting changes for
straker
previously requested changes
Aug 23, 2021
Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com>
stephenmathieson
approved these changes
Aug 23, 2021
straker
added a commit
that referenced
this pull request
Aug 24, 2021
This PR sets the axe reporters up so they can run without access to the top-level browsing context. This is needed so that `axe.finishRun()` can be called outside the context of a web page. Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com> Co-authored-by: Stephen Mathieson <me@stephenmathieson.com> Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR sets the axe reporters up so they can run without access to the top-level browsing context. This is needed so that
axe.finishRun()can be called outside the context of a web page.