-
Notifications
You must be signed in to change notification settings - Fork 15
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
Add support in import test script for importing test harness #1064
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@evmiguel great stuff! Definitely looking forward to getting this in and reducing the overhead involved with certain w3c/aria-at
updates! I've left a few inline thoughts but this seems very close.
This also made me notice w3c/aria-at
is missing some changes which this would now remove so I've also submitted w3c/aria-at#1056. This work made that disconnect super clear.
@@ -125,3 +125,5 @@ server/migrations/test_plan_target_id.csv | |||
|
|||
# Private Key files (installed by deploy) | |||
jwt-signing-key.pem | |||
|
|||
client/resources |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since there are still updates to client/resources
in this PR, you'll also need to untrack client/resources
by doing: git rm -r --cached client/resources
, and push again afterwards.
Although, I am now wondering if this should need to be done? It will require that contributors be somewhat aware of when last changes to the w3c/aria-at
files have happened so they may re-run the import script locally. Otherwise there could be confusion if some w3c/aria-at
-dependent operations aren't working as expected if the client application's files are updated but not the now ignored aria-at harness changes.
My above concern is relatively minor though and seems like a docs/local-development.md
update could be useful here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a brief notes in the docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! After untracking the client/resources
folder, this PR should be good to go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All set!
This PR adds support for importing the test harness into the ARIA-AT App repo from the ARIA-AT repo. To test, run:
Verify that the harness files are in
client/resources
. Additionally, this PR sets thecommands.json
inserver/resources
tocommandsV1.json
. My intent was to make the import minimally invasive. Open to other thoughts!