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

Reference files content-only change in aria-at#1173 not creating new version after test import #1304

Open
IsaDC opened this issue Jan 22, 2025 · 4 comments
Assignees
Labels
help wanted Extra attention is needed nvda NVDA related issues question Further information is requested

Comments

@IsaDC
Copy link

IsaDC commented Jan 22, 2025

Context:

PR #1173 resolved an issue that was blocking NVDA tests from running. However, the PR only updated the test example and not the corresponding test plan.

Issue:

Due to the test plan not being modified in PR #1173, it's currently impossible to update the test plan to a new version.

Question:

Is there a way to quickly push an update to the test plan to address this issue?
CC @howard-e @jugglinmike
Thanks!

Related PR: ARIA-AT #1173

@IsaDC IsaDC added help wanted Extra attention is needed question Further information is requested nvda NVDA related issues changes-requested Used by issues created from the ARIA-AT App and removed changes-requested Used by issues created from the ARIA-AT App labels Jan 22, 2025
@howard-e
Copy link
Contributor

@IsaDC just to confirm, this is about w3c/aria-at#1173? And not the linked aria-at-app's #1173? As what was linked above points to an accessibility issue that doesn't impact tests from running.

@howard-e
Copy link
Contributor

@IsaDC after properly checking out w3c/aria-at#1173 instead like I was assuming, I'm now sure the issue is indeed there so no need to confirm!

A "quick" way this could be addressed is to rename the reference folders used in both tests/link-css/references/ and tests/link-img-alt/references/ then update the reference value cell in both tests/link-css/data/references.csv and tests/link-img-alt/data/references.csv.

The reason it didn't trigger a version bump as is, is because the contents of those files aren't being directly referenced on import, rather the files are being pointed to and rendered when needed.

I'm not well versed on the expectations around the frequency and need for those reference folder renames as I wasn't present when that was built. But it feels like this is a use case it should cover (a folder name update with content update -- even when not freshly imported). I'm only speculating on that though, so if there's a request to trigger a version bump with just an update to a references folder's content, we can also track that for the future!

@IsaDC
Copy link
Author

IsaDC commented Jan 23, 2025

@IsaDC after properly checking out w3c/aria-at#1173 instead like I was assuming, I'm now sure the issue is indeed there so no need to confirm!

A "quick" way this could be addressed is to rename the reference folders used in both tests/link-css/references/ and tests/link-img-alt/references/ then update the reference value cell in both tests/link-css/data/references.csv and tests/link-img-alt/data/references.csv.

The reason it didn't trigger a version bump as is, is because the contents of those files aren't being directly referenced on import, rather the files are being pointed to and rendered when needed.

I'm not well versed on the expectations around the frequency and need for those reference folder renames as I wasn't present when that was built. But it feels like this is a use case it should cover (a folder name update with content update -- even when not freshly imported). I'm only speculating on that though, so if there's a request to trigger a version bump with just an update to a references folder's content, we can also track that for the future!

@IsaDC after properly checking out w3c/aria-at#1173 instead like I was assuming, I'm now sure the issue is indeed there so no need to confirm!

A "quick" way this could be addressed is to rename the reference folders used in both tests/link-css/references/ and tests/link-img-alt/references/ then update the reference value cell in both tests/link-css/data/references.csv and tests/link-img-alt/data/references.csv.

The reason it didn't trigger a version bump as is, is because the contents of those files aren't being directly referenced on import, rather the files are being pointed to and rendered when needed.

I'm not well versed on the expectations around the frequency and need for those reference folder renames as I wasn't present when that was built. But it feels like this is a use case it should cover (a folder name update with content update -- even when not freshly imported). I'm only speculating on that though, so if there's a request to trigger a version bump with just an update to a references folder's content, we can also track that for the future!

Thank you @howard-e!
Would you mind pushing those changes to the test plan at your end? I focused on rebuilding the test plans with the existing examples, rather than pulling new ones, so I didn't think renaming the "references" folder was necessary in my PR.

@howard-e
Copy link
Contributor

howard-e commented Jan 23, 2025

Discussed in 2025-01-23 CG Meeting today, test admins will move forward with a suitable rename which forces the the app to pick up the import.

The app team will investigate further how we can allow for these "reference content only" changes to be picked up and create a new import without any additional tweaks for the future.

sharing above to use this as the tracking issue

@howard-e howard-e self-assigned this Jan 23, 2025
@howard-e howard-e changed the title Unable to Update Test Plan After PR #1173 Reference files content-only change in aria-at#1173 not creating new version after test import Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed nvda NVDA related issues question Further information is requested
Development

No branches or pull requests

2 participants