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

Wait for theme-directives.html E2E test to render before taking a screenshot #4846

Merged

Conversation

aloisklink
Copy link
Member

📑 Summary

The theme-directives.html test currently sometimes takes a screenshot before all of the Mermaid diagrams have completed rendering.

This can result in the following error message when running pnpm run e2e:

1) Configuration and directives - nodes should be light blue
when rendering several diagrams
  diagrams should not taint later diagrams:
Error: Image was 23.384242424242423% different from saved snapshot with 154336 different pixels.
See diff for details: /mermaid/cypress/snapshots/rendering/conf-and-directives.spec.js/__diff_output__/conf-and-directives.spec-when-rendering-several-diagrams-diagram-1.diff.png
at Context.eval (webpack:///./node_modules/.pnpm/cypress-image-snapshot@4.0.1_cypress@12.10.0_jest@29.5.0/node_modules/cypress-image-snapshot/command.js:51:0)

conf-and-directives spec-when-rendering-several-diagrams-diagram-1 diff

Notes to reviewer

I'm not sure why we're only detecting this issue now in our E2E tests. Maybe #4759 changed some performance parameters, and that made the race-condition occur on my PC. Who knows, it could also just be because one of our dependencies updated, or now my PC is overheating and is running slightly slower 😆

Also, because this is just a test change, I've labelled this PR as Skip changelog, since there's no point in putting it in the release notes.

📋 Tasks

Make sure you

@aloisklink aloisklink added the Skip changelog Don't include in the changelog label Sep 15, 2023
@netlify
Copy link

netlify bot commented Sep 15, 2023

Deploy Preview for mermaid-js ready!

Name Link
🔨 Latest commit 17f5052
🔍 Latest deploy log https://app.netlify.com/sites/mermaid-js/deploys/65073e00eb846900085b9b92
😎 Deploy Preview https://deploy-preview-4846--mermaid-js.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@codecov
Copy link

codecov bot commented Sep 15, 2023

Codecov Report

Merging #4846 (17f5052) into develop (4201e47) will decrease coverage by 0.85%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #4846      +/-   ##
===========================================
- Coverage    80.55%   79.70%   -0.85%     
===========================================
  Files          148      148              
  Lines        13024    13024              
  Branches       612      612              
===========================================
- Hits         10491    10381     -110     
- Misses        2400     2510     +110     
  Partials       133      133              
Flag Coverage Δ
e2e 84.75% <ø> (-1.05%) ⬇️
unit 43.67% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 12 files with indirect coverage changes

Copy link
Member

@sidharthv96 sidharthv96 left a comment

Choose a reason for hiding this comment

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

aloisklink and others added 2 commits September 17, 2023 18:55
We don't use any font-awesome icons, or custom fonts in this file.
The `theme-directives.html` test currently sometimes takes a screenshot
before all of the Mermaid diagrams have completed rendering.

We can use the `urlSnapshopTest()` helper function, which waits until
a `.rendered` property exists on the page.

Co-authored-by: Sidharth Vinod <sidharthv96@gmail.com>
@aloisklink aloisklink force-pushed the test/fix-theme-directives-e2e-test branch from 15ab657 to 17f5052 Compare September 17, 2023 17:57
@aloisklink
Copy link
Member Author

#4847 (review)

Fixed by 17f5052 to use the urlSnapshotTest() helper function! I've also removed some of the unused code in the theme-directives.html file, since I was making modifications to it anyway.

The only difference is that the snapshot now has a different ID, it's now located at cypress/snapshots/conf-and-directives.spec.js/Configuration-and-directives---nodes-should-be-light-blue-when-rendering-several-diagrams-diagrams-should-not-taint-later-diagrams.snap.png and looks like:

Configuration-and-directives---nodes-should-be-light-blue-when-rendering-several-diagrams-diagrams-should-not-taint-later-diagrams snap

@sidharthv96 sidharthv96 added this pull request to the merge queue Sep 19, 2023
Merged via the queue into mermaid-js:develop with commit e8347ce Sep 19, 2023
18 checks passed
@aloisklink aloisklink deleted the test/fix-theme-directives-e2e-test branch September 19, 2023 11:26
fuxingloh pushed a commit to fuxingloh/contented that referenced this pull request Nov 3, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [mermaid](https://togithub.com/mermaid-js/mermaid) | [`10.5.1` ->
`10.6.0`](https://renovatebot.com/diffs/npm/mermaid/10.5.1/10.6.0) |
[![age](https://developer.mend.io/api/mc/badges/age/npm/mermaid/10.6.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/mermaid/10.6.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/mermaid/10.5.1/10.6.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/mermaid/10.5.1/10.6.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>mermaid-js/mermaid (mermaid)</summary>

###
[`v10.6.0`](https://togithub.com/mermaid-js/mermaid/releases/tag/v10.6.0):
10.6.0

[Compare
Source](https://togithub.com/mermaid-js/mermaid/compare/v10.5.1...v10.6.0)

#### What's Changed

- Add new chart xychart by
[@&#8203;subhash-halder](https://togithub.com/subhash-halder) in
[mermaid-js/mermaid#4413

#### Fix

- bug/4849\_center_axis_labels by
[@&#8203;dreathed](https://togithub.com/dreathed) in
[mermaid-js/mermaid#4860
- Better handling of large flowcharts and long edges
[@&#8203;knsv](https://togithub.com/knsv)

#### Docs

- Add new Atlassian integrations by
[@&#8203;janjonas](https://togithub.com/janjonas) in
[mermaid-js/mermaid#4862
- docs: fix typo by
[@&#8203;dennis0324](https://togithub.com/dennis0324) in
[mermaid-js/mermaid#4887
- Update notes on orientation in GitGraph documentation by
[@&#8203;guypursey](https://togithub.com/guypursey) in
[mermaid-js/mermaid#4897
- Enhancment: twitter logo in doc by
[@&#8203;chaursiyasanjeet](https://togithub.com/chaursiyasanjeet) in
[mermaid-js/mermaid#4925
- Update link for the Mermaid integration in JetBrains IDEs by
[@&#8203;FirstTimeInForever](https://togithub.com/FirstTimeInForever) in
[mermaid-js/mermaid#4883

#### Chores

- Wait for `marker_unique_id.html` E2E test to render before taking a
screenshot by [@&#8203;aloi](https://togithub.com/aloi)

sklink[mermaid-js/mermaid#4847
- Wait for `theme-directives.html` E2E test to render before taking a
screenshot by [@&#8203;aloisklink](https://togithub.com/aloisklink) in
[mermaid-js/mermaid#4846
- chore(deps): update all patch dependencies (patch) by
[@&#8203;renovate](https://togithub.com/renovate) in
[mermaid-js/mermaid#4851
- chore(dev-deps): update `@typescript-eslint/*` plugins to v6 (major)
by [@&#8203;aloisklink](https://togithub.com/aloisklink) in
[mermaid-js/mermaid#4857
- chore: shorten `flow-huge.spec.js` test case using `.repeat` by
[@&#8203;Yokozuna59](https://togithub.com/Yokozuna59) in
[mermaid-js/mermaid#4859
- Publish Live Editor previews for the `develop` & `next` branches by
[@&#8203;sidharthv96](https://togithub.com/sidharthv96) in
[mermaid-js/mermaid#4841
- chore(deps): update all minor dependencies (minor) by
[@&#8203;renovate](https://togithub.com/renovate) in
[mermaid-js/mermaid#4870
- chore(deps): update all patch dependencies (patch) by
[@&#8203;renovate](https://togithub.com/renovate) in
[mermaid-js/mermaid#4869
- Commented out broken test by
[@&#8203;nirname](https://togithub.com/nirname) in
[mermaid-js/mermaid#4913
- chore(deps): update all patch dependencies (patch) by
[@&#8203;renovate](https://togithub.com/renovate) in
[mermaid-js/mermaid#4891
- fix(class): avoid duplicate definition of fill by
[@&#8203;Mister-Hope](https://togithub.com/Mister-Hope) in
[mermaid-js/mermaid#4929
- chore(deps): update all minor dependencies (minor) by
[@&#8203;renovate](https://togithub.com/renovate) in
[mermaid-js/mermaid#4892
- making consitent config imports from diagramAPI by
[@&#8203;dreathed](https://togithub.com/dreathed) in
[mermaid-js/mermaid#4889
- fix(typos): Fix minor typos in the source code by
[@&#8203;mribeirodantas](https://togithub.com/mribeirodantas) in
[mermaid-js/mermaid#4928
- chore(deps): update all patch dependencies (patch) by
[@&#8203;renovate](https://togithub.com/renovate) in
[mermaid-js/mermaid#4945
- Bump [@&#8203;babel/traverse](https://togithub.com/babel/traverse)
from 7.22.10 to 7.23.2 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[mermaid-js/mermaid#4951
- Replace rehype-mermaidjs with rehype-mermaid by
[@&#8203;remcohaszing](https://togithub.com/remcohaszing) in
[mermaid-js/mermaid#4970

#### New Contributors

- [@&#8203;dreathed](https://togithub.com/dreathed) made their first
contribution in
[mermaid-js/mermaid#4860
- [@&#8203;janjonas](https://togithub.com/janjonas) made their first
contribution in
[mermaid-js/mermaid#4862
- [@&#8203;dennis0324](https://togithub.com/dennis0324) made their first
contribution in
[mermaid-js/mermaid#4887
- [@&#8203;FirstTimeInForever](https://togithub.com/FirstTimeInForever)
made their first contribution in
[mermaid-js/mermaid#4883
- [@&#8203;guypursey](https://togithub.com/guypursey) made their first
contribution in
[mermaid-js/mermaid#4897
- [@&#8203;chaursiyasanjeet](https://togithub.com/chaursiyasanjeet) made
their first contribution in
[mermaid-js/mermaid#4925
- [@&#8203;mribeirodantas](https://togithub.com/mribeirodantas) made
their first contribution in
[mermaid-js/mermaid#4928

**Full Changelog**:
mermaid-js/mermaid@v10.5.1...v10.6.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/levaintech/contented).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zMS41IiwidXBkYXRlZEluVmVyIjoiMzcuMzEuNSIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip changelog Don't include in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants