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

fix: prevent infinite loop on prettyDOM calls #7250

Merged
merged 34 commits into from
Jan 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
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: 6 additions & 0 deletions packages/browser/src/node/providers/playwright.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,12 @@ export class PlaywrightBrowserProvider implements BrowserProvider {
})
}

// unhandled page crashes will hang vitest process
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 was making the tests hang #7249 (comment)

page.on('crash', () => {
const session = this.project.vitest._browserSessions.getSession(sessionId)
session?.reject(new Error('Page crashed when executing tests'))
})

return page
}

Expand Down
3 changes: 2 additions & 1 deletion packages/utils/src/display.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,9 @@ export function stringify(
})
}

// Prevents infinite loop https://github.com/vitest-dev/vitest/issues/7249
return result.length >= MAX_LENGTH && maxDepth > 1
? stringify(object, Math.floor(maxDepth / 2))
? stringify(object, Math.floor(Math.min(maxDepth, Number.MAX_SAFE_INTEGER) / 2), { maxLength, ...options })
: result
}

Expand Down
12 changes: 12 additions & 0 deletions test/browser/fixtures/browser-crash/browser-crash.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { commands } from '@vitest/browser/context'
import { it } from 'vitest'

declare module '@vitest/browser/context' {
interface BrowserCommands {
forceCrash: () => Promise<void>
}
}

it('fails gracefully when browser crashes', async () => {
await commands.forceCrash()
})
39 changes: 39 additions & 0 deletions test/browser/fixtures/browser-crash/vitest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { fileURLToPath } from 'node:url'
import { defineConfig } from 'vitest/config'
import { instances, provider } from '../../settings'
import { BrowserCommand } from 'vitest/node'

const forceCrash: BrowserCommand<[]> = async (context) => {
const browser = context.context.browser().browserType().name()
if (browser === 'chromium') {
await context.page.goto('chrome://crash')
}

if (browser === 'firefox') {
await context.page.goto('about:crashcontent')
}

throw new Error(`Browser crash not supported for ${browser}`)
}

export default defineConfig({
cacheDir: fileURLToPath(new URL("./node_modules/.vite", import.meta.url)),
test: {
browser: {
commands: { forceCrash },
enabled: true,
provider,
instances: instances.map(instance => ({
...instance,
context: {
actionTimeout: 500,
},
})),
},
expect: {
poll: {
timeout: 500,
},
},
},
})
17 changes: 17 additions & 0 deletions test/browser/specs/browser-crash.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { expect, test } from 'vitest'
import { instances, provider, runBrowserTests } from './utils'

// TODO handle webdriverio. Currently they
// expose no trustable way to detect browser crashes.
test.runIf(provider === 'playwright')('fails gracefully when browser crashes', async () => {
const { stderr } = await runBrowserTests({
root: './fixtures/browser-crash',
reporters: [['verbose', { isTTY: false }]],
browser: {
// webkit has no support for simulating browser crash
instances: instances.filter(item => item.name !== 'webkit'),
},
})

expect(stderr).contains('Page crashed when executing tests')
})
10 changes: 10 additions & 0 deletions test/browser/test/__snapshots__/utils.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,13 @@ exports[`prints the element with attributes 1`] = `
</div>
</body>"
`;

exports[`should handle DOM content bigger than maxLength 1`] = `
"<body>
<div>
<div>
<div … />
</div>
</div>
</body>..."
`;
15 changes: 15 additions & 0 deletions test/browser/test/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,18 @@ test('prints the element with attributes', async () => {

expect(await commands.stripVTControlCharacters(prettyDOM())).toMatchSnapshot()
})

test('should handle DOM content bigger than maxLength', async () => {
const depth = 200
const maxContent = 150

const openingTags = '<div>'.repeat(depth)
const closingTags = '</div>'.repeat(depth)
const domString = `${openingTags}${closingTags}`

const parentDiv = document.createElement('div')
parentDiv.innerHTML = domString

document.body.appendChild(parentDiv)
expect(await commands.stripVTControlCharacters(prettyDOM(undefined, maxContent))).toMatchSnapshot()
})
Loading