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

fix(raw-reporter): do not output DqElements #1513

Merged
merged 2 commits into from
Apr 26, 2019

Conversation

stephenmathieson
Copy link
Member

@stephenmathieson stephenmathieson commented Apr 24, 2019

This patch updates our "raw" reporter, ensuring it does not pass DqElements to its callback. Instead, it serializes the DqElement by using the DqElement#toJSON() method.

Unfortunately many of our tests pass non-results to the reporters, so several "guards" were added to the reporter ensuring we don't error while attempting to transform the provided results. In an ideal world, these tests would be rewritten to pass "proper" result objects to our reporters, but this felt out-of-scope for the task at hand.

Closes #1195

Reviewer checks

Required fields, to be filled out by PR reviewer(s)

  • Follows the commit message policy, appropriate for next version
  • Has documentation updated, a DU ticket, or requires no documentation change
  • Includes new tests, or was unnecessary
  • Code is reviewed for security by: @WilcoFiers

This patch updates our "raw" reporter, ensuring it does not pass `DqElement`s to its callback. Instead, it serializes the `DqElement` by using the `DqElement#toJSON()` method.

Unfortunately many of our tests pass non-results to the reporters, so several "guards" were added to the reporter ensuring we don't error while attempting to transform the provided results. In an ideal world, these tests would be rewritten to pass "proper" result objects to our reporters, but this felt out-of-scope for the task at hand.

Closes #1195
@stephenmathieson stephenmathieson marked this pull request as ready for review April 24, 2019 17:06
@stephenmathieson stephenmathieson requested a review from a team as a code owner April 24, 2019 17:06
Copy link
Contributor

@WilcoFiers WilcoFiers left a comment

Choose a reason for hiding this comment

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

Did you mean to check in the playground?

@stephenmathieson
Copy link
Member Author

Did you mean to check in the playground?

It was "dirty" and I just did git commit -am .... I'm not sure what the "rules" are here. Should I roll back the changes?

@jeeyyy
Copy link
Contributor

jeeyyy commented Apr 25, 2019

Yup, the rule is, playground.html is something that is not committed, so yes rollback the playground changes.

@jeeyyy
Copy link
Contributor

jeeyyy commented Apr 25, 2019

Which is why I thought perhaps playground.html should be bare bones as it got overridden by this PR - https://github.com/dequelabs/axe-core/pull/1446/files#diff-fef561d00f83fd7586addaf3f391c681

And I wanted to roll it back as per this PR #1511 (but there were concerns).

@WilcoFiers
Copy link
Contributor

@stephenmathieson Jup, just roll it back.

@stephenmathieson
Copy link
Member Author

OK, I'll roll it back.

What's the purpose of versioning the file if it's not supposed to be updated tho?

@stephenmathieson stephenmathieson dismissed WilcoFiers’s stale review April 26, 2019 15:06

Reverted playground stuff

@stephenmathieson stephenmathieson merged commit 3babcb6 into develop Apr 26, 2019
@stephenmathieson stephenmathieson deleted the fix-raw-reporter-dqelement branch April 26, 2019 16:33
stephenmathieson added a commit to mohanraj-r/axe-core that referenced this pull request May 10, 2019
* develop: (50 commits)
  docs: add jsdom example and tests (dequelabs#1530)
  fix(aria-valid-attr-value): allow aria-owns to pass when element is not in the DOM (dequelabs#1526)
  fix(isSkipLink): cache first page link (dequelabs#1525)
  chore: update Babel dependencies (dequelabs#1527)
  chore(package): Update karma to version 4.1.0 (dequelabs#1528)
  feat: Improve perf of axe.run [WWD-1821] (dequelabs#1503)
  fix(prettier): ignore generated api doc files (dequelabs#1522)
  fix(skip-link,region): Allow multiple skiplinks at page top (dequelabs#1496)
  fix(raw-reporter): do not output `DqElement`s (dequelabs#1513)
  fix: Scroll state had top and left properties flipped (dequelabs#1469)
  refactor: commons.color.getBackgroundColor method (dequelabs#1451)
  fix(aria-valid-attr-value): allow aria-controls to pass when element is not in the DOM
  chore: Update husky to the latest version 🚀 (dequelabs#1514)
  style: format HTML files with Prettier (dequelabs#1508)
  test: Fix invalid test html (dequelabs#1502)
  feat(rule): Inline text spacing must be adjustable with custom stylesheets (dequelabs#1446)
  chore: Remove version number from axe.d.ts (dequelabs#1499)
  chore: Update make-dir to the latest version 🚀 (dequelabs#1465)
  fix: Exclude  iframe for html-has-lang rule (Issue 1424) (dequelabs#1430)
  feat(utils): add support for complex CSS selectors (dequelabs#1494)
  ...
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.

raw reporter shouldn't output DqElements
3 participants