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

Added ITestOutputHelper to A11y tests #170

Merged
merged 1 commit into from Feb 13, 2024
Merged

Conversation

WolfyUK
Copy link
Collaborator

@WolfyUK WolfyUK commented Feb 13, 2024

Context

HTTP errors due to misconfigured A11y tests were being consumed by the application making debugging difficult.

Change proposed in this pull request

Output HTTP responses to the ITestOutputHelper to give better visibility over what the A11y tests are doing.

Guidance to review

Running the A11y tests locally should output all HTTP responses.

Checklist

  • [ ] Work items have been linked (use AB#)
  • Your code builds clean without any errors or warnings
  • You have run all unit/integration tests and they pass
  • Your branch has been rebased onto main
  • You have tested by running locally

Chris-Lorch
Chris-Lorch previously approved these changes Feb 13, 2024
@jrabbott
Copy link
Collaborator

This doesn't feel right. If we're wanting to set the output helper in the WebDrover then I'd suggest not set it as a IClassFixture.

Instead I would pass the output helper explicitly in the constructor for the WebDriver. Then follow constructor/disposal pattern for the test class.

https://xunit.net/docs/shared-context

Copy link
Collaborator

@jrabbott jrabbott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't be use IClassFixture for WebDriver if we need to set dependencies.

@WolfyUK
Copy link
Collaborator Author

WolfyUK commented Feb 13, 2024

We shouldn't be use IClassFixture for WebDriver if we need to set dependencies.

Doing in the IClassFixture doesn't work - it seems as though this is the only pattern that does:

Happy to swap out IClassFixture completely if that is deemed acceptable?

@WolfyUK WolfyUK marked this pull request as draft February 13, 2024 10:37
@WolfyUK WolfyUK marked this pull request as ready for review February 13, 2024 10:56
Copy link
Collaborator

@jrabbott jrabbott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could probably clean up the page assessment, but something for a future change.

@WolfyUK WolfyUK merged commit 7462074 into main Feb 13, 2024
8 checks passed
@WolfyUK WolfyUK deleted the tech-debt/a11y-tests-output branch February 13, 2024 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants