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

Large img preview in self-contained mode #573

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mganisin
Copy link

"ful-screen" preview of extra images attached to results for self-contained mode. Unlike image preview in normal mode implemented as part of page with little bit of js+css (likely also possible to implement via , however adress line populated by base64 of the image would be really ugly in such case).

"ful-screen" preview of extra images attached to results for
self-contained mode. Unlike image preview in normal mode implemented as
part of page with little bit of js+css (likely also possible to
implement via <a>, however adress line populated by base64 of the image
would be really ugly in such case).
@BeyondEvil
Copy link
Contributor

Thanks for the PR!

Since we're about to release the first RC of v4 (aka next-gen) which is a complete rewrite of the plugin, let's put this on hold.

One of the things we've implemented in v4 is an image gallery, and I think this has been solved there.

@BeyondEvil
Copy link
Contributor

Can you try 4.0.0rc0 and see if it works as expected?

@mganisin
Copy link
Author

mganisin commented Apr 13, 2023

Can you try 4.0.0rc0 and see if it works as expected?

Could be a breaking change there? I get:

INTERNALERROR>     extra.append(pytest_html.extras.url(issue, name=issue_id))
INTERNALERROR> AttributeError: 'function' object has no attribute 'url'

@BeyondEvil
Copy link
Contributor

Can you try 4.0.0rc0 and see if it works as expected?

Could be a breaking change there? I get:

INTERNALERROR>     extra.append(pytest_html.extras.url(issue, name=issue_id))
INTERNALERROR> AttributeError: 'function' object has no attribute 'url'

Can you try ”from pytest_html import extras” instead?

@mganisin
Copy link
Author

Can you try 4.0.0rc0 and see if it works as expected?

Could be a breaking change there? I get:

INTERNALERROR>     extra.append(pytest_html.extras.url(issue, name=issue_id))
INTERNALERROR> AttributeError: 'function' object has no attribute 'url'

Can you try ”from pytest_html import extras” instead?

Right, I found the problem (I used pluginmanager to get pytest-html). This doesn't seem to be only breaking change, At least report.extra is renamed to report.extras without backward compatibility (my preference is to not break interfaces, but I understand that not everyone shares this approach).

screenshots in self-contained mode don't seem to work at all (same code used with old and new version, got these results):

  1. src attribute of <img> is different. current version adds data:image/png;base64,${BASE64} version 4.0.0 adds just ${BASE64}. Preview screenshot is not displayed in new version (while properly displayed in old)
  2. Clicking the preview (which is not displayed) opens URL with ${BASE64} included (I tried this as well when working on images preview as this is easiest way to do that, but found base64 in URL pretty inconvenient; if I remember properly, my attempt worked, maybe because of data:... prefix which is missing in 4.0.0). That results in error:
Request-URI Too Long

The requested URL's length exceeds the capacity limit for this server.

Footnote: Personally I don't appreciate new data-jsonblob. pytest-html is amazing also because of the speed. This won't improve the speed (not to mention that such result is unprocessable).

@BeyondEvil
Copy link
Contributor

This is awesome feedback, thank you! 🙏

Can you try ”from pytest_html import extras” instead?

Right, I found the problem (I used pluginmanager to get pytest-html). This doesn't seem to be only breaking change, At least report.extra is renamed to report.extras without backward compatibility (my preference is to not break interfaces, but I understand that not everyone shares this approach).

Correct. I'm exploring ways of keeping the "pluginmanager" functionality. But I'd like to at least warn when it's used. That type of usage makes future changes difficult. The proper way is to use what I recommended, a regular import.

As for "report.extra", that should be backwards compatible, but throws a DeprecationWarning when used. Could you please share how it's breaking for you?

screenshots in self-contained mode don't seem to work at all (same code used with old and new version, got these results):

  1. src attribute of <img> is different. current version adds data:image/png;base64,${BASE64} version 4.0.0 adds just ${BASE64}. Preview screenshot is not displayed in new version (while properly displayed in old)
  2. Clicking the preview (which is not displayed) opens URL with ${BASE64} included (I tried this as well when working on images preview as this is easiest way to do that, but found base64 in URL pretty inconvenient; if I remember properly, my attempt worked, maybe because of data:... prefix which is missing in 4.0.0). That results in error:
Request-URI Too Long

The requested URL's length exceeds the capacity limit for this server.

I'm having trouble reproducing at least 1. In my testing I get previews in both regular and self-contained mode. Would you mind sharing some reproducible example code/test?

Footnote: Personally I don't appreciate new data-jsonblob. pytest-html is amazing also because of the speed. This won't improve the speed (not to mention that such result is unprocessable).

Would you mind elaborating on this a little bit?

How many tests does your report contain? Do all of them have extras in the form of images? In what way is the report slow? Rendering? Scrolling? etc.

Also, as for the unprocessable bit, what are you doing today that isn't possible with the new report?

My intent with v4 is that it should strive to be 1-to-1 with the old report as mush as possible. I'm trying my best to make as much as possible backwards compat. For things I want to change/remove I'm doing DeprecationWarnings so that I in v5 can remove them.

If you check the docs, all deprecations should be listed and have suggested mitigations.

Again, thank you for input - it's super valuable! 🙏

@BeyondEvil
Copy link
Contributor

Proper v4 is released now if you're interested in revisiting this @mganisin ?

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.

None yet

2 participants