Skip to content

Video support for mp4 format #260

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

Merged
merged 3 commits into from
Feb 5, 2020

Conversation

ExaltedBagel
Copy link
Contributor

@ExaltedBagel ExaltedBagel commented Feb 2, 2020

Summary

This feature enables the use of extra videos in the reports in the same way as images are used.

extra.video(content, name="Video", mime_type="video/mp4", extension="mp4")
extra.mp4(content, name="Video")

Details

  • Videos may be included as reference, or as standalone
  • Both of these cases are managed as the images are. Base64 encoding is used to pack the video as standalone data if needed.
  • Extra video and windows path for videos are both added to the unit tests.
  • Video and images were refactored to use a common media function internally to eliminate code duplication.

Support uses the raw html section in order to pass a video tag which is compatible with html5.

Video size and style should be handled by the "video" class in the css file.
…duplicate of the image css.

Code for the video extra has been extracted to a function to clean up a bit the if/elif/elif that is starting to
get pretty big. Maybe the other conditions could be extracted to their own method to clean up a bit. Once again,
there seems to be a lot of code duplication with image, so we may want to refactor images, videos, and audio as
a media type and have some kind of template method.

Basic unit tests were added to make sure that the extra was correctly added to the report when the extra was
requested.
BeyondEvil
BeyondEvil previously approved these changes Feb 2, 2020
Copy link
Contributor

@BeyondEvil BeyondEvil left a comment

Choose a reason for hiding this comment

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

Looks really good, thanks for this!

Would you mind creating an issue re. creating a media type (instead of separate image and video)?

…st-dev#260. These changes aimed at generelizing the

approach used to introduce media types to the project. Images and videos now use a method called
_make_media_html_div which generates the media div in a streamlined manner. Both _append_image and _append_video
use this method, to which they can pass their base string and class.

Unit tests were left untouched, as the changes only affect private methods.
Copy link
Contributor

@BeyondEvil BeyondEvil left a comment

Choose a reason for hiding this comment

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

Again, looks really good! Thanks!

@ExaltedBagel
Copy link
Contributor Author

I think I would be done for this feature. Feel free to merge whenever you are ready! I might grab another issue in the meantime such as #245

@BeyondEvil BeyondEvil merged commit 2513821 into pytest-dev:master Feb 5, 2020
@robbiewareham
Copy link

This is awesome! Any date on when the new release will be built?

@BeyondEvil
Copy link
Contributor

I'm aiming to do a release during the weekend @robbiewareham

@BeyondEvil
Copy link
Contributor

Released in v2.1.0

@BeyondEvil
Copy link
Contributor

Hey @ExaltedBagel !

I discovered a bug with your PR.

On this line, href will always be none in the case of FORMAT_IMAGE.

Just wanted you to know, and I might ask you to review the fix. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This issue/PR relates to a feature request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants