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.

Sign up for GitHub

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

fix: prevent infinite loop on prettyDOM calls #7250

Merged
merged 34 commits into from
Jan 16, 2025
Merged

fix: prevent infinite loop on prettyDOM calls #7250

Changes from 1 commit
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
2605182
fix: prevent infinite loop on prettyDOM calls
tsirlucas Jan 14, 2025
f780722
fix lint
tsirlucas Jan 14, 2025
0b13cbf
use MAX_SAFE_INTEGER instead of MAX_VALUE
tsirlucas Jan 15, 2025
6be46fe
add test case to handle large nested DOM in prettyDOM calls
tsirlucas Jan 15, 2025
f815b0a
handle browser crash in playwright provider
tsirlucas Jan 15, 2025
9b02f20
fix lint issues
tsirlucas Jan 15, 2025
ed16cda
bump test counter
tsirlucas Jan 15, 2025
0841caf
Merge branch 'main' into main
tsirlucas Jan 15, 2025
34ff688
adjust test count
tsirlucas Jan 15, 2025
7c00ff2
Merge branch 'main' of https://github.com/tsirlucas/vitest
tsirlucas Jan 15, 2025
c0b9c08
rebump test count
tsirlucas Jan 15, 2025
4e1adfc
Merge branch 'main' into main
tsirlucas Jan 15, 2025
e2f36ac
fix browser crash test
tsirlucas Jan 15, 2025
3a343d3
Merge branch 'main' of https://github.com/tsirlucas/vitest
tsirlucas Jan 15, 2025
8ee78bc
force maxDepth to be under predefined value
tsirlucas Jan 15, 2025
4d1728d
bump test counter
tsirlucas Jan 15, 2025
32a967f
update snapshot
tsirlucas Jan 15, 2025
9c2f84d
update test counters
tsirlucas Jan 15, 2025
2189730
add bigger timeout to large dom test
tsirlucas Jan 15, 2025
3bcb162
increase large dom test timeout
tsirlucas Jan 15, 2025
69299a2
exit gracefully on webdriver crash
tsirlucas Jan 15, 2025
8f6f9a6
Merge branch 'main' into main
tsirlucas Jan 15, 2025
d88b2a1
fix lint
tsirlucas Jan 15, 2025
f10d107
Merge branch 'main' of https://github.com/tsirlucas/vitest
tsirlucas Jan 15, 2025
fc4f64a
better crashing strategy
tsirlucas Jan 15, 2025
346c2ef
dont test crash on webkit
tsirlucas Jan 15, 2025
39bc395
undo webdriver changes
tsirlucas Jan 15, 2025
77b8972
remove unneeded code
tsirlucas Jan 15, 2025
1a90e07
better assertion for prettyDOM test
tsirlucas Jan 15, 2025
0002e1b
update maxContent for readability
tsirlucas Jan 15, 2025
891036d
adjust test so test counter is not dynamic
tsirlucas Jan 15, 2025
ed80dc4
use different depth and maxContent values for better snapshots
tsirlucas Jan 16, 2025
6307327
use test.runIf in browser crashing test
tsirlucas Jan 16, 2025
90600e8
pr review fix
tsirlucas Jan 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions packages/utils/src/display.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,11 @@
...options,
})
}


Check failure on line 75 in packages/utils/src/display.ts

View workflow job for this annotation

GitHub Actions / Lint: node-latest, ubuntu-latest 

Trailing spaces not allowed
// Prevents infinite loop https://github.com/vitest-dev/vitest/issues/7249
const nextMaxDepth = maxDepth === Infinity ? Number.MAX_VALUE : Math.floor(maxDepth / 2)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think here instead of Number.MAX_VALUE we could go with result.length. The max possible depth in the DOM will never be bigger than the length of its stringified content. But Im afraid Im missing some edge case (e.g. custom maxLength may mess things up) so Number.MAX_VALUE is safer approach IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is stil a pretty big recursion so I wonder how it will behave in CI env with limited resources. Will run a test and get back here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I have pretty similar results time-wise doesnt matter if I use Number.MAX_VALUE or just 10.

return result.length >= MAX_LENGTH && maxDepth > 1
? stringify(object, Math.floor(maxDepth / 2))
? stringify(object, nextMaxDepth)
: result
}

Expand Down
Loading