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

api(video): simplify video api #3924

Merged
merged 1 commit into from
Sep 19, 2020
Merged

api(video): simplify video api #3924

merged 1 commit into from
Sep 19, 2020

Conversation

dgozman
Copy link
Contributor

@dgozman dgozman commented Sep 18, 2020

  • This leaves just recordVideos and videoSize options on the context.
  • Videos are saved to artifactsPath. We also save their file names to trace.
  • context.close() waits for the processed videos.

Copy link
Member

@yury-s yury-s left a comment

Choose a reason for hiding this comment

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

The API where you cannot attribute video to its page is somewhat weird but as discussed offline let's keep it this way until we have a compelling use case.

if (pageOrError instanceof Page)
makeWaitForNextTask()(() => pageOrError.emit(Page.Events.VideoStarted, video));
Copy link
Member

Choose a reason for hiding this comment

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

This was necessary for the following code to work:

const page = await context.newPage();
const v = await page.waitForEvent('videostarted');

Do we not need this to work any more?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we don't have an event anymore, so this is not a problem currently.

src/server/firefox/ffPage.ts Outdated Show resolved Hide resolved
This leaves just `recordVideos` and `videoSize` options on the context.
Videos are saved to `artifactsPath`. We also save their ids to trace.
@dgozman dgozman merged commit df77734 into microsoft:master Sep 19, 2020
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 this pull request may close these issues.

3 participants