tests: update #registerExternalDiagrams testTimeout from 5 seconds to 20 seconds

[omer-priel]

📑 Summary

Brief description about the content of your PR.

resolves #5053

📏 Design Decisions

Describe the way your implementation works or what design decisions you made if applicable.

📋 Tasks

Make sure you

• 📖 have read the contribution guidelines
• 💻 have added necessary unit/e2e tests.
• 📓 have added documentation. Make sure MERMAID_RELEASE_VERSION is used for all new features.
• 🔖 targeted develop branch

[netlify]

✅ Deploy Preview for mermaid-js ready!

Name	Link
🔨 Latest commit	9ae00eb
🔍 Latest deploy log	https://app.netlify.com/sites/mermaid-js/deploys/655f4bc10a4b120008f880e8
😎 Deploy Preview	https://deploy-preview-5055--mermaid-js.netlify.app
📱 Preview on mobile	Toggle 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]

Codecov Report

Merging #5055 (9ae00eb) into develop (4a4e614) will increase coverage by 36.70%.
The diff coverage is n/a.

Additional details and impacted files

@@             Coverage Diff              @@
##           develop    #5055       +/-   ##
============================================
+ Coverage    42.91%   79.61%   +36.70%     
============================================
  Files           23      164      +141     
  Lines         5029    13869     +8840     
  Branches        21      698      +677     
============================================
+ Hits          2158    11042     +8884     
+ Misses        2870     2677      -193     
- Partials         1      150      +149     

Flag	Coverage Δ
e2e	85.59% <ø> (?)
unit	42.91% <ø> (ø)

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

see 160 files with indirect coverage changes

[aloisklink]

Awesome work @omer-priel, I've randomly encountered the same issue on my tests, but I didn't expect it to be fixed by just increasing the timeout.

I don't think we should be updating the test timeout for all tests.

Instead, can you update the test timeout for each test individually?

E.g. adding the new timeout in:

• mermaid/packages/mermaid/src/mermaid.spec.ts

  Line 92 in 4a4e614

  });

• mermaid/packages/mermaid/src/mermaid.spec.ts

  Line 151 in 4a4e614

  });

• mermaid/packages/mermaid/src/mermaidAPI.spec.ts

  Line 673 in 4a4e614

  });

• mermaid/packages/mermaid/src/mermaidAPI.spec.ts

  Line 690 in 4a4e614

  });

All of the it() functions in vitest support adding a timeout as their last param, see https://vitest.dev/api/#test, so we should just need to change them to }, 10_000); (or 20_000).

Maybe we should also bump the timeout to 20s? If it's taking more than 5seconds for you and me, I'm sure it'll be even slower on older PCs.

[omer-priel]

Thanks, I will update this

[omer-priel]

Sometimes it passed the test without change the timeout and sometimes it fails
I dont know why?
If the timeout is 20 seconds (or 10 seconds) it always passed the test

[nirname]

@omer-priel I think you are the first one who are really digging into the problem. It would be great if you could provide a comprehensive answer to your own question and share it with the community. May be it is due to some async code that is used as a synchronous one. But that is just a blind guess. Increasing timeout without understanding the nature of the issue is a temporary patch. So I can agree even on 1 minute wait if it would guarantee that never again we would have to increase that value

[omer-priel]

I can learn more and try to find the root of this problem. But, if the timeout be less then 1 minute. Then this problem not worth the time.
And i prefer used this time for other bugs

[nirname]

Yep, you are absolutely right. 20 seconds is more or less OK as a temporary solution. If there is a high chance of failures regardless of the timeout then it's not worth it. But if slight increase helps significantly then we are good to go. I think everybody can put up with a few more seconds of waiting instead of addressing the same question "why my tests fail" over and over in Slack.

[omer-priel]

If in the future it will return i will fix it :)

[aloisklink]

Looks good to me :)

Normally I'll keep this PR open for other people to review, but since this PR is very small, low risk, and will have a big benefit for everybody, I'll merge this now!

---

Sometimes it passed the test without change the timeout and sometimes it fails
I dont know why?

I suspect the issue might be to do with Vite.

Maybe these tests are waiting for Vite to build something and on slower computers (or maybe slow computers with multiple cores?), Vite is still building something during these tests and it fails.

It would maybe explain why the tests only fail the first time you run tests, since the next time might have some cached data.

We are planning on moving away from Vite to ESBuild in the next Mermaid v11 release (see #4729), and ESBuild is much faster, so maybe this issue will fix itself in the future 🤷

[mermaid-bot]

@omer-priel, Thank you for the contribution!
You are now eligible for a year of Premium account on MermaidChart.
Sign up with your GitHub account to activate.