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

Improve Percy + accessibilityAudit test coverage on commits Page #34960

Merged
merged 4 commits into from May 9, 2022

Conversation

gitstart-sourcegraph
Copy link
Collaborator

@gitstart-sourcegraph gitstart-sourcegraph commented May 5, 2022

Description

We currently have visual regression + accessibility audit tests on a series of user journeys within Sourcegraph. A lot of core functionality is still not covered, it reduces our confidence in shipping changes as UI/accessibility bugs can be introduced without our awareness.

Implementation

For this Commits Page

  1. Add percySnapshotWithVariants to store a visual snapshot for the test.
  2. Add accessibilityAudit to run our accessibility tests, and fix any identified problems.

Refs

Sourcegraph Issue
Gitstart Ticket

Test plan

Run ENTERPRISE=1 yarn build-web
Run ENTERPRISE=1 HEADLESS=true yarn _test-integration client/web/src/integration/repository.test.ts
Test should pass successfully

App preview:

Check out the client app preview documentation to learn more.

@cla-bot cla-bot bot added the cla-signed label May 5, 2022
@gitstart-sourcegraph gitstart-sourcegraph self-assigned this May 5, 2022
@gitstart-sourcegraph gitstart-sourcegraph added the gitstart Contract partner label May 5, 2022
@gitstart-sourcegraph gitstart-sourcegraph linked an issue May 5, 2022 that may be closed by this pull request
@gitstart-sourcegraph gitstart-sourcegraph changed the title [SG-34403-5] Improve Percy + accessibilityAudit test coverage on commits Page Improve Percy + accessibilityAudit test coverage on commits Page May 5, 2022
@@ -154,6 +154,7 @@ export const RepositoryCommitsPage: React.FunctionComponent<React.PropsWithChild
history={props.history}
hideSearch={true}
location={props.location}
listComponent="div"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
listComponent="div"

What is the failure here? is it because there is no

  • element present?

    I think we still ideally want this to be a list, as it definitely helps understand each item (e.g. commit 10 out of 20)

  • Copy link
    Contributor

    @umpox umpox left a comment

    Choose a reason for hiding this comment

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

    Thanks! Visual diff looks good, left some small accessibility comments

    Copy link
    Contributor

    @umpox umpox left a comment

    Choose a reason for hiding this comment

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

    Great - thank you!

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    cla-signed gitstart Contract partner
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    Improve Percy + accessibilityAudit test coverage
    3 participants