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

e2e are not started when only documentation changes #2364

Closed
imagoiq opened this issue Dec 6, 2023 · 4 comments
Closed

e2e are not started when only documentation changes #2364

imagoiq opened this issue Dec 6, 2023 · 4 comments
Labels
needs: refinement on hold Wait for something to be done, before continue working on this Issue ⚙️ setup Setup work for the repository or a new package

Comments

@imagoiq
Copy link
Contributor

imagoiq commented Dec 6, 2023

e2e test were not triggered on CI with #2175
Pnpm did not trigger the internet-header tests because there were no change there and there is no dependency for the documentation on the internet-header packages. But in reality, there is a dependency between the two as the Cypress test are using the documentation to test the component.

We could add the documentation as a dependency in the package.json, but this creates a loop as the documentation is using the internet-header package to display the component.

So we should either:

  • Move the e2e internet-header tests to a separate package
  • Move the e2e internet-header tests inside the documentation package
  • Change the e2e tests to not rely on the documentation.
  • Force e2e tests to run when documentation change
@gfellerph gfellerph added the ⚙️ setup Setup work for the repository or a new package label Dec 6, 2023
@gfellerph
Copy link
Member

gfellerph commented Dec 6, 2023

The third point would probably be the cleanest solution. We could easily use fixture html files like we already use for the language tests for all other header tests.

The components suffer from the same issue. There, for example for the popover tests, the component is embedded in the storybook iframe which puts a div.overflow-hidden around the button that triggers the popover. The popover is then displayed "outside" of the div area and cypress thinks it's not visible and fails the .should('be.visible) test.

If only component testing for stencil would work (cypress-io/cypress#24054)...

As a bonus, this would make the tests faster because we don't have to compile the docs before the test. As a malus, we have to maintain separate markup for test cases.

gfellerph added a commit that referenced this issue Dec 6, 2023
Storybook stories are rendered inside some wrapper elements, one of which has overflow: hidden set. For this component, that's a problem because cypress then thinks the popover is not visible as it overlaps the containers bounds. Using a fixture creates the most simple environment possible and increases test stability. Also, see #2364.
@imagoiq
Copy link
Contributor Author

imagoiq commented Dec 7, 2023

Let's think about it and make a decision on the next triage

@gfellerph
Copy link
Member

Decided to go with option 3

@imagoiq imagoiq added on hold Wait for something to be done, before continue working on this Issue needs: refinement labels Feb 28, 2024
@gfellerph
Copy link
Member

Closing as won't fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs: refinement on hold Wait for something to be done, before continue working on this Issue ⚙️ setup Setup work for the repository or a new package
Projects
Development

No branches or pull requests

2 participants