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

[Feature] Redirect video stream to new path #5853

Closed
thernstig opened this issue Mar 17, 2021 · 4 comments · Fixed by #6005
Closed

[Feature] Redirect video stream to new path #5853

thernstig opened this issue Mar 17, 2021 · 4 comments · Fixed by #6005
Assignees

Comments

@thernstig
Copy link
Contributor

Some of us run Playwright as test cases, e.g. via Jest using it statements and test files (suites). When using the awesome videos functionality it saves one huge video in case the browser context never closes between tests.

It would be cool to have hooks to page.video() e.g. page.video().path('some-path') to redirect the buffer to a new file. It would also be cool if I can chose the file name of the video in those scenarios, as I might want to name them as the test case. (I can add a UUID or similar myself if wanted).

@pavelfeldman
Copy link
Member

in case the browser context never closes between tests

The problem is this ^^, you should always create a new context for each test.

@thernstig
Copy link
Contributor Author

@pavelfeldman Doesn't a new page make more sense per test? A new context does not sound sensible. E.g. https://github.com/playwright-community/jest-playwright keeps one context per "test file". But each "test file" has multiple it statements (multiple tests).

Your own examples also have one context for many tests: https://playwright.dev/docs/test-runners

No matter I think the user case to let the user redirect the Buffer to another file at any point is a powerful feature. Then you leave it up to the user how to split videos.

@pavelfeldman
Copy link
Member

I would strongly encourage you to use context per test. The test can have several steps, but still context per story, not page per story. If you deviate from this strategy, it'll be increasingly hard to use Playwright long term - all our decisions are around isolation and efficiency.

I'm keeping this as a result for page.video().saveAs() though since it is orthogonal to the context discussion and makes sense!

@thernstig
Copy link
Contributor Author

@pavelfeldman Quick aside. In terms of e.g. Jest, do you consider the test file or the it/test statement to have one context each? The later could be problematic with load times since you need to open a new context + page + loading the page, which could take time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants