Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(runPartial): Test without frame communication #3006

Merged
merged 12 commits into from
Jul 1, 2021
Merged

Conversation

WilcoFiers
Copy link
Contributor

@WilcoFiers WilcoFiers commented Jun 16, 2021

This adds two new methods to the axe object. axe.runPartial runs axe in the current window, and collects information about its iframes. axe.finishRun takes information from multiple axe.runPartial calls from different windows, builds a report from it.

Closes #2936

@WilcoFiers WilcoFiers changed the base branch from develop to create-frame-context June 29, 2021 15:31
Base automatically changed from create-frame-context to develop June 29, 2021 16:09
Copy link
Contributor

@straker straker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far looking good

})
.then(data => {
// TODO: Proper copy with .toJSON call instead of this;
const results = JSON.parse(JSON.stringify(data));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why copy the results from data if we're just returning it?

import { getReporter } from './reporter';
import { mergeResults, publishMetaData, finalizeRuleResult, DqElement } from '../utils';

export default function finishRun(data, options = {}) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wasn't clear what data was in this function, had to read the tests to understand it was an array of runPartial results.

).then(function (partialResults) {
return Promise.all([
axe.finishRun(partialResults, options),
axe.run(clone(context), options)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was confused why you were running axe.run here. Looking further into the test I see you're making sure they are the same result. Might be better if this sort of test was first defined like:

"it(should give the same results as axe.run)"

function runPartial(
context: ElementContext,
options: RunOptions
): Promise<unknown>;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Techdebt ticket here: #3059

@WilcoFiers WilcoFiers changed the title Run partial feat(runPartial): Test without frame communication Jul 1, 2021
@WilcoFiers WilcoFiers marked this pull request as ready for review July 1, 2021 12:47
@WilcoFiers WilcoFiers requested a review from a team as a code owner July 1, 2021 12:47
var attrName = attrs[i].name;
if (attrName !== 'id') {
fixture.removeAttribute(attrs[i].name);
if (typeof beforeEach !== 'undefined' && typeof afterEach !== 'undefined') {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put this in so that we can use testutils outside a test run. I was using it in playground.

@WilcoFiers WilcoFiers merged commit 42738b5 into develop Jul 1, 2021
@WilcoFiers WilcoFiers deleted the run-partial branch July 1, 2021 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New runPartial and finishRun methods
2 participants