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

bug: emulateis not used for E2E Screenshot tests #5353

Closed
3 tasks done
mlechler opened this issue Feb 12, 2024 · 8 comments · Fixed by #5359
Closed
3 tasks done

bug: emulateis not used for E2E Screenshot tests #5353

mlechler opened this issue Feb 12, 2024 · 8 comments · Fixed by #5359
Assignees
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil

Comments

@mlechler
Copy link

mlechler commented Feb 12, 2024

Prerequisites

Stencil Version

4.12.1

Current Behavior

We have four viewports defined in the emulate array in the stencil.config.ts. When running our e2e screenshot tests these defined viewports aren't used. As mentioned here, I understood, that the emulate array should still be used for screenshot tests, which isn't the case.

Expected Behavior

Use the emulate array of the stencil.config.ts or provide a easy way to use multiple viewports for every e2e screenshot test, without setting every single one of it.

System Info

System: node 20.11.0
    Platform: darwin (21.6.0)
   CPU Model: Apple M1 Pro (10 cpus)
    Compiler: /node_modules/@stencil/core/compiler/stencil.js
       Build: 1707148558
     Stencil: 4.12.1 🏸
  TypeScript: 5.3.3
      Rollup: 2.56.3
      Parse5: 7.1.2
      jQuery: 4.0.0-pre
      Terser: 5.27.0

Steps to Reproduce

Run npm run test.screenshot in the test repo.
Should generate screenshot with 600x600, but generates 800x600

Code Reproduction URL

https://github.com/mlechler/stencil-e2e-test-bug

Additional Information

No response

@tanner-reits tanner-reits self-assigned this Feb 12, 2024
@tanner-reits
Copy link
Member

@mlechler I confirmed that the generated screenshots do not have the same dimensions as what is being set in the Stencil config. I'll get this ingested in our backlog for someone to take a look at.

@tanner-reits tanner-reits added Bug: Validated This PR or Issue is verified to be a bug within Stencil and removed triage labels Feb 12, 2024
@mlechler
Copy link
Author

Hi @tanner-reits, what is the time horizon on this issue?

@tanner-reits
Copy link
Member

@mlechler I don't have a definitive timeline right now. I spent some more time playing around with a fix, but we uncovered some additional discrepancies in the generated output. The team needs to dig in deeper to the screenshot testing process so we're confident in the change(s) we make. This is an experimental feature in Stencil after all, so there are many kinks and nuances we need to work through.

@christian-bromann
Copy link
Member

christian-bromann commented Mar 6, 2024

@mlechler thanks to @tanner-reits work we were able to publish a dev release with a fix for this bug. You can install it in your project via:

npm i @stencil/[email protected]

Can you verify that this resolves the issue?

@mlechler
Copy link
Author

mlechler commented Mar 7, 2024

Hi @christian-bromann, the viewport works great now! 🥳

I just found one more problem in our use case, which worked earlier.

We are using the userAgent property of the emulateConfig to emulate different themes (color & spacing), e.g.
{ userAgent: 'Dark', viewport: { width: 600, height: 600, } }
Afterwards we are setting different attributes to the root <html> based on the userAgent to emulate light/dark themes etc.

In our tests I can see that the userAgent property is not used, as the userAgent of the test is Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/116.0.5814.0 Safari/537.36 instead of Dark.

Is this an additional bug or is it intended to work like that? If it is intended: How can we emulate different themes (color & spacing) in our screenshot tests?

@christian-bromann
Copy link
Member

the viewport works great now! 🥳

Awesome, I will go ahead and close the issue.

Is this an additional bug or is it intended to work like that? If it is intended: How can we emulate different themes (color & spacing) in our screenshot tests?

Mind raising a new issue for this?

github-merge-queue bot pushed a commit that referenced this issue Mar 11, 2024
…5359)

* fix(testing): use viewport for Puppeteer screenshot clip dimensions

Fixes #5353

STENCIL-1135

* export options function and test

* pr feedback
@mlechler
Copy link
Author

Great thanks @christian-bromann!

I opened #5459 for the new issue.

@christian-bromann
Copy link
Member

The fix has been released with Stencil 🚞 v4.13.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants