w3c / aria-at

Assistive Technology ARIA Experience Assessment
https://aria-at.netlify.app
Other
154 stars 28 forks source link

Introduce automated tests #1105

Closed howard-e closed 3 months ago

howard-e commented 3 months ago

Preview Tests

This introduces tests which compare the snapshots of a 4 specific test plans which have been pulled into a dedicated __mocks__ folder:

This PR does significant restructuring of the create-all-tests and the review-tests scripts to make the results of those overall scripts testable (ie. content that gets added into the /build folder).

Additional tests can follow this PR to verify the many utilities and the harness output used in this project. To avoid ballooning this PR further, we can introduce this initial work and do the aforementioned tests as subsequent PRs to make future reviews easier.

Note: Also confirmed these changes don't cause any critical issues when being imported into aria-at-app. (build log)

howard-e commented 3 months ago
  1. createReviewPages.mjs feels like it could be broken up a little to make it easier to read. Some sections become deeply nested.
  2. The logging during testing is difficult to parse. I wonder if this could be helped with something simple like an emphasis color on the summary messages? This isn't essential but could be nice.

Good point on both, will take a pass at making that content easier to parse

howard-e commented 3 months ago
  1. createReviewPages.mjs feels like it could be broken up a little to make it easier to read. Some sections become deeply nested.

@stalgiag I added several commits to break up the createReviewPages.mjs into several smaller components. Let me know me know your thoughts there!

  1. The logging during testing is difficult to parse. I wonder if this could be helped with something simple like an emphasis color on the summary messages? This isn't essential but could be nice.

https://github.com/w3c/aria-at/pull/1105/commits/b33362270bd9f7684a37f9c45415c9bdb8a05c5c should address this. Please let me know if this aligns with what you were thinking. Also curious if you think this functionality should be set behind the test flag?

howard-e commented 3 months ago

@stalgiag thanks for the feedback! Found a presumably long standing issue (but presentational) with how the assertionExceptions logic is handled in when built with the json data that I've addressed in https://github.com/w3c/aria-at/pull/1105/commits/c66455fec3847d9e7ad887848fe0676663023988. The gist being that json filtered view could show all the assertionExceptions if excluded in assertions.csv instead of *-commands.csv. The collected json view (which is also what the app uses) didn't have this issue.

screenshot showing json filtered view incorrectly showing 'jaws switched from virtual cursor active to pc cursor active' for 'down down' command ![json filtered view incorrectly showing 'jaws switched from virtual cursor active to pc cursor active' for 'down down' command](https://github.com/user-attachments/assets/347896a4-1c27-4c3e-a173-5e99cae3486a)
screenshot showing collected json view correctly excluding 'jaws switched from virtual cursor active to pc cursor active' for 'down down' command ![collected json view correctly excluding 'jaws switched from virtual cursor active to pc cursor active' for 'down down' command](https://github.com/user-attachments/assets/60d42bfe-f627-4ee3-b18c-ff4c60ed496f)

Also re-confirmed these changes don't cause any critical issues when being imported into aria-at-app. (build log)