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

Adding ability to post violations via middleware #212

Merged
merged 5 commits into from
Sep 9, 2020

Conversation

scalvert
Copy link
Contributor

@scalvert scalvert commented Sep 4, 2020

Adds support for combining the custom reporter with custom middleware, which allows for serializing the results to JSON.

// tests/test-helper.js
import { setupMiddlewareReporter } from 'ember-a11y-testing';
// ...

setupMiddlewareReporter();

// ...

This will setup a custom reporter, which gathers the test module name, test name, and violations for each failed audit. Once the suite is done, these results are posted to the testem middleware, ultimately serializing the results to disc.

TODO:

  • Add tests for setupMiddlewareReporter
  • Update setupMiddleware test to use real AxeResults
  • Determine which specific directory path to pass to setupMiddlewareReporter in order to save the results.

@scalvert scalvert force-pushed the reporter-middleware branch from 7e64401 to ab33aa6 Compare September 4, 2020 19:51
node-tests/fixtures/violations.json Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
setup-middleware.js Outdated Show resolved Hide resolved
@drewlee
Copy link
Contributor

drewlee commented Sep 4, 2020

Just as a general question, since I didn't see this directly referred to in the code - how does the host application determine whether they want to run tests w/reporter vs no reporter (i.e., conditionally)? For example, we may not need the reporter for CI/CD process or local execution, while requiring the reporter for a nightly job. Would we just wrap setupMiddlewareReporter() in a conditional check in tests/test-helper.js?

@scalvert scalvert changed the title WIP: Adding ability to post violations via middleware Adding ability to post violations via middleware Sep 5, 2020
@scalvert
Copy link
Contributor Author

scalvert commented Sep 6, 2020

@drewlee - yes. I think we can configure that in the test-helper.js file via a custom query param.

@scalvert
Copy link
Contributor Author

scalvert commented Sep 8, 2020

@rwjblue WDYT about us moving the parallel execution support (ember-exam) work to a subsequent PR? This will allow us to ship this increment, and be able to build out the subsequent supporting parts later.

@scalvert scalvert force-pushed the reporter-middleware branch from a6de773 to 3501bde Compare September 8, 2020 16:54
@scalvert scalvert force-pushed the reporter-middleware branch from 3501bde to 7ea81e0 Compare September 8, 2020 16:58
setup-middleware.js Show resolved Hide resolved
node-tests/setup-middleware-test.js Show resolved Hide resolved
@scalvert scalvert requested a review from rwjblue September 9, 2020 16:56
Comment on lines +66 to +76
serverMiddleware(startOptions) {
setupMiddleware(startOptions.app, {
root: this.project.root,
});
},

testemMiddleware(app) {
setupMiddleware(app, {
root: this.project.root,
});
},
Copy link
Member

Choose a reason for hiding this comment

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

The differences in these methods (from ember-cli's side) is very annoying 😭

setup-middleware.js Show resolved Hide resolved
@rwjblue rwjblue merged commit cf0d122 into ember-a11y:master Sep 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants