Introduce automated tests #1105
Conversation
There was a problem hiding this comment.
Read through all of the changes and everything looks good! Also ran the test and love this new, easier entry point for interacting with the repo. The only two comments that I have are both non-blocking, nice-to-haves.
createReviewPages.mjsfeels like it could be broken up a little to make it easier to read. Some sections become deeply nested.- 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.
| @@ -5,7 +5,12 @@ on: | |||
|
|
|||
| jobs: | |||
| generate-and-commit-files: | |||
| runs-on: ubuntu-latest | |||
| runs-on: ${{ matrix.os }} | |||
|
|
||
| // For V2 to handle assertion exceptions | ||
| assertionsForCommandsInstructions = assertionsForCommandsInstructions.map( | ||
| (assertionForCommand, assertionForCommandIndex) => { |
There was a problem hiding this comment.
This section is feeling deeply nested and it becomes a little tricky to follow. Perhaps there can be a dedicated function for processing each of the collectedTestsData? This could break it up a little bit and make it easier to follow.
Good point on both, will take a pass at making that content easier to parse |
@stalgiag I added several commits to break up the
b333622 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? |
There was a problem hiding this comment.
Wow you went above and beyond! Thanks for addressing the feedback. The new createReviewPages organization is much easier for me to follow with smart separation of responsibilities.
The log styling is great and I like it as-is. With scripts that produce lots of output, like this one, it is useful to err on the side of conveying more contextual/severity info where possible.
…figKeysTitleUnexpected. This doesn't affect the app because it uses fromCollectedTestCommandsKeysUnexpected
|
@stalgiag thanks for the feedback! Found a presumably long standing issue (but presentational) with how the screenshot showing json filtered view incorrectly showing 'jaws switched from virtual cursor active to pc cursor active' for 'down down' command
screenshot showing collected json view correctly excluding 'jaws switched from virtual cursor active to pc cursor active' for 'down down' command
Also re-confirmed these changes don't cause any critical issues when being imported into aria-at-app. (build log) |
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-testsand thereview-testsscripts to make the results of those overall scripts testable (ie. content that gets added into the/buildfolder).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)