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

Internal: add Cypress integration tests #1220

Merged
merged 4 commits into from Sep 17, 2020

Conversation

christianvuerings
Copy link
Contributor

@christianvuerings christianvuerings commented Sep 16, 2020

Instead of Netlify CLI, use GitHub actions to run cypress integration tests.

Test Plan

  • GitHub Integration Tests action pass

Next steps

Notes

  • Cypress does not have flow-typed definitions so we are ignoring the directory for now.
  • We use the official Cypress GitHub Action: https://github.com/cypress-io/github-action.
  • Tests will only be recorded when we push to master (otherwise we might leak the Cypress key)

@christianvuerings christianvuerings requested a review from a team as a code owner September 16, 2020 20:44
@netlify
Copy link

netlify bot commented Sep 16, 2020

Deploy preview for gestalt ready!

Built with commit da806d1

https://deploy-preview-1220--gestalt.netlify.app

@netlify
Copy link

netlify bot commented Sep 16, 2020

Deploy preview for gestalt ready!

Built with commit 71724fe

https://deploy-preview-1220--gestalt.netlify.app

@@ -0,0 +1,7 @@
{
"baseUrl": "http://localhost:3000",
"projectId": "x99ctf",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

See https://dashboard.cypress.io/projects/x99ctf - plan to add this link to our docs once this is merged

@@ -16,7 +16,7 @@ card(
<Link inline target="blank" href="https://npmjs.org/package/gestalt">
<img
src="https://img.shields.io/npm/v/gestalt.svg?label=gestalt"
alt="Gestalt's NPM package version badge"
alt="Gestalt NPM package version badge"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A11Y fix - axe-core picked up that we had the same descriptions for these links

mdPadding={6}
lgPadding={8}
width="100%"
role="main"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A11Y fix - we need to add role to all places that have content on the page

<Link
href={`#${slugifiedId}`}
inline
accessibilityLabel={`${name} - Anchor tag`}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A11Y fix: used to be an empty link without a good label

@@ -1,5 +1,5 @@
.Markdown pre {
background-color: rgb(236, 236, 236);
background-color: rgba(236, 236, 236, 0.5);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A11Y fix - detected that the contrast wasn't high enough (WCAG 2.0 AA)

@@ -10,8 +10,8 @@ export default function SidebarSection({
section: sidebarIndexType,
|}): Node {
return (
<>
<Box padding={2} marginTop={4} role="list">
<Box role="list">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A11Y Fix: we previous had a <Box role="list" /> without any role="listitem" in them.

@@ -50,20 +53,20 @@
"lint-staged": "^10.3.0",
"netlify-cli": "^2.61.2",
"papaparse": "^5.3.0",
"postcss": "^7.0.32",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

alphabetized these (happened when I added cypress as a dev package)

@christianvuerings christianvuerings added the minor release Minor release label Sep 17, 2020
@christianvuerings christianvuerings merged commit c95caad into pinterest:master Sep 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor release Minor release
Projects
None yet
2 participants