Auto Enable A11y fix #1023
Auto Enable A11y fix #1023
Conversation
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
bin/testhub/testhubHandler.js
Outdated
| const userAccessibilityEnabled = testhubUtils.isAccessibilityEnabled(user_config); | ||
| const serverAutoEnabled = testhubUtils.isAccessibilityInResponse(response.data); | ||
|
|
||
| console.log('[A11Y] C# SDK pattern check:', { |
There was a problem hiding this comment.
Can we remove 'c# SDK' from the log or is it intentional
There was a problem hiding this comment.
yes, should be removed thanks for pointing this out
| } else { | ||
| // If accessibility is undefined, keep default to null | ||
| logger.debug('[A11Y] isAccessibilityEnabled from config: accessibility is undefined, returning null'); | ||
| return null; |
There was a problem hiding this comment.
if we early return null from here then environment variable wont be checked, is that expected?
|
|
||
| // Also store scriptsToRun if available | ||
| if (commandsToWrapData.scriptsToRun) { | ||
| process.env.ACCESSIBILITY_SCRIPTS_TO_RUN = JSON.stringify(commandsToWrapData.scriptsToRun); |
There was a problem hiding this comment.
we are not using this
| }; | ||
|
|
||
| // Get accessibility script by name | ||
| exports.getAccessibilityScript = (scriptName) => { |
There was a problem hiding this comment.
this is unused remove
There was a problem hiding this comment.
made the change
| ...payloadToSend, | ||
| scanMode: isBuildEndOnlyMode ? "comprehensive-build-end" : "incremental", | ||
| timestamp: Date.now(), | ||
| serverScripts: Cypress.env('ACCESSIBILITY_SCRIPTS') || null |
There was a problem hiding this comment.
is this required, it would be a large payload
There was a problem hiding this comment.
made the changes
| }); | ||
|
|
||
| // Store server scripts for Cypress to read | ||
| process.env.ACCESSIBILITY_SCRIPTS = JSON.stringify(scriptsMap); |
| @@ -0,0 +1,227 @@ | |||
| const path = require('path'); | |||
There was a problem hiding this comment.
scripts are hardcoded do we need this?
| const serverCommands = commandsToWrapData.commands || []; | ||
|
|
||
| // Store server commands for Cypress to read | ||
| process.env.ACCESSIBILITY_COMMANDS_TO_WRAP = JSON.stringify(serverCommands); |
There was a problem hiding this comment.
these need to be added at setSystemEnvs, else won't be available in browserstack terminal
Maintain something like OBSERVABILITY_ENV_VARS for a11y as well
There was a problem hiding this comment.
made the changes
Uh oh!
There was an error while loading. Please reload this page.