-
Notifications
You must be signed in to change notification settings - Fork 158
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
test(table-of-contents): add e2e-storybook test coverage #7673
test(table-of-contents): add e2e-storybook test coverage #7673
Conversation
Deploy preview created for package Built with commit: dcfa294573ffe718b6d2d7ddfed8f38c9f64feaf |
Deploy preview created for package Built with commit: dcfa294573ffe718b6d2d7ddfed8f38c9f64feaf |
Deploy preview created for package Built with commit: dcfa294573ffe718b6d2d7ddfed8f38c9f64feaf |
Deploy preview created for package Built with commit: dcfa294573ffe718b6d2d7ddfed8f38c9f64feaf |
Deploy preview created for package Built with commit: dcfa294573ffe718b6d2d7ddfed8f38c9f64feaf |
Deploy preview created for package Built with commit: dcfa294573ffe718b6d2d7ddfed8f38c9f64feaf |
Deploy preview created for package Built with commit: dcfa294573ffe718b6d2d7ddfed8f38c9f64feaf |
...omponents/tests/e2e-storybook/cypress/integration/table-of-contents/table-of-contents.e2e.js
Show resolved
Hide resolved
@jeffchew #7444 has the react package tag on it, but there's already a TOC test in the react packages e2e-storybook directory. Does it need touched? |
@andy-blum yeah we should be updating tests for react as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you @andy-blum
/** | ||
* Cycle through carbon themes and take a screenshot | ||
*/ | ||
Cypress.Commands.add('carbonScreenshot', (screenshotOpts = {}) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesomeee!!! just small name change
Cypress.Commands.add('carbonScreenshot', (screenshotOpts = {}) => { | |
Cypress.Commands.add('carbonThemesScreenshot', (screenshotOpts = {}) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jeffchew I just removed the ready to merge label for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Going to re-run the e2e tests for web components, looks like the table of contents tests failed. |
@andy-blum the tests appear to almost be done but seeing some weird error: |
Curious if that's from the Percy snapshot command. Do you know where that's defined? |
'should remain visible on page throughout scroll', | ||
_tests.mobile.checkStickyNav | ||
); | ||
it('should render correctly in all themes', _tests.all.screenshotThemes); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andy-blum I think this is the issue, didn't you rename the method to carbonThemesScreenshot
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
talked to @andy-blum on slack and the method is calling carbonThemesScreenshot above, but i think it might be the width
causing the error https://github.com/carbon-design-system/carbon-for-ibm-dotcom/pull/7673/files#diff-a42d6ed5ec53b43667f81c546bfc457893f5157db7a2bddd00eef414126fc929R35
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix pushed up. 🤞
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the react e2e-storybook tests are failing now 😞
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re-ran and it passed!!! Looks like this one is good to go :D
Related Ticket(s)
Closes: #7444 #7445 #7446
Description
Changelog
New
yarn test:e2e-storybook:dev
that opens the Cypress client on a currently-running storybook instance to ease test writing & debuggingtable of contents
component