fix: allow chain call timeout to override nested items timeout

[hainenber]

PR Checklist

• Addresses an existing open issue: fixes 🐛 Bug: timeout() chain call on describe not working as expected #5422
• That issue was marked as status: accepting prs
• Steps in CONTRIBUTING.md were taken

Overview

Fixes the issue with timeout chain call in describe suite doesn't correctly override the config for inner/nested suite and test cases.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.41%. Comparing base (0ed524a) to head (a589e40).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5612   +/-   ##
=======================================
  Coverage   88.40%   88.41%           
=======================================
  Files          66       66           
  Lines        4787     4791    +4     
  Branches      977      977           
=======================================
+ Hits         4232     4236    +4     
  Misses        555      555           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[mark-wiemer]

hmm, I'm not sure how timeout(70) and timeout(100) result in different behavior when we have setTimeout(resolve, 55). Would replacing 70 with 40 clarify things? Do we need two tests for this?

Some code comments might be useful here :)

[hainenber]

The chained timeout calls are meant to imply that the outermost timeout value would override for nested test cases and suites. In general, timeout(100) would take precedence over other timeout configs.

We'll definitely need 2 test to illustrate for nested and deeply nested cases :D

[hainenber]

I've added code comments for clarity, that is if sufficient.

[hainenber]

I think I'll extend the timeout a bit higher than the default value, 2 second, to illustrate the fix better. Con is that the test will be slightly longer.

Edit: Done

[lennonnikolas]

I'm a little new to the codebase and I'm sorry if I'm out of line here but it seems like these are arbitrary values that indicate a deeper root issue and this seems to be a bandaid fix?

[mark-wiemer]

No worries. These are arbitrary values in some sample tests to demonstrate that the new sleep values do actually apply. The backing bug was reported because in main*, with something like describe( ... ).timeout(100);, the timeout(100) call didn't do anything at all.

While I haven't reviewed the latest iterations, the goal is to write a test that only passes if timeout(...) works as designed on both describe and test.

But this isn't a bandaid fix, because these are just sample tests :)

[lennonnikolas]

Makes sense now :)