-
-
Notifications
You must be signed in to change notification settings - Fork 394
feat: add OpenDocument thumbnail support (port #366) #545
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
Conversation
Co-Authored-By: Josh Beatty <joshuatb6@gmail.com>
def test_odt_preview(cwd, snapshot): | ||
file_path: Path = cwd / "fixtures" / "sample.odt" | ||
renderer = ThumbRenderer() | ||
img: Image.Image = renderer._open_doc_thumb(file_path) | ||
|
||
img_bytes = io.BytesIO() | ||
img.save(img_bytes, format="PNG") | ||
img_bytes.seek(0) | ||
assert img_bytes.read() == snapshot(extension_class=PNGImageSnapshotExtension) | ||
|
||
|
||
def test_ods_preview(cwd, snapshot): | ||
file_path: Path = cwd / "fixtures" / "sample.ods" | ||
renderer = ThumbRenderer() | ||
img: Image.Image = renderer._open_doc_thumb(file_path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these two tests (and the tests in #543 #540 #539 ) can be eventually DRY-ed by using pytest.mark.parametrize
. It executes the test with values in the supplied list, which works great in case the only thing which changes is the fixture name (and thumbnailing method in case of the other PRs). So it can look like this:
def test_odt_preview(cwd, snapshot): | |
file_path: Path = cwd / "fixtures" / "sample.odt" | |
renderer = ThumbRenderer() | |
img: Image.Image = renderer._open_doc_thumb(file_path) | |
img_bytes = io.BytesIO() | |
img.save(img_bytes, format="PNG") | |
img_bytes.seek(0) | |
assert img_bytes.read() == snapshot(extension_class=PNGImageSnapshotExtension) | |
def test_ods_preview(cwd, snapshot): | |
file_path: Path = cwd / "fixtures" / "sample.ods" | |
renderer = ThumbRenderer() | |
img: Image.Image = renderer._open_doc_thumb(file_path) | |
@pytest.mark.parametrize(["fixture_file", "thumbnailer"], [ | |
("sample.odt", ThumbRenderer._open_doc_thumb), # _open_doc_thumb needs to be changed to @classmethod | |
("sample.ods", ThumbRenderer._open_doc_thumb), # _open_doc_thumb needs to be changed to @classmethod | |
]) | |
def test_document_preview(cwd, fixture_file, thumbnailer, snapshot): | |
file_path: Path = cwd / "fixtures" / fixture_file | |
img: Image.Image = thumbnailer(file_path) |
so instead of two almost-identical tests, there will be just one, and the fixture file + thumbnailing method is passed inside via the parameters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these two tests (and the tests in #543 #540 #539 ) can be eventually DRY-ed by using
pytest.mark.parametrize
. It executes the test with values in the supplied list, which works great in case the only thing which changes is the fixture name (and thumbnailing method in case of the other PRs). So it can look like this:so instead of two almost-identical tests, there will be just one, and the fixture file + thumbnailing method is passed inside via the parameters.
I've tried merging the tests as shown here, including marking open_doc_thumb
as a @classmethod
, however it seems that the snapshots can not be found causing the tests to fail. Here's how I have the test set up:
@pytest.mark.parametrize(
["fixture_file", "thumbnailer"],
[
(
"sample.odt",
ThumbRenderer._open_doc_thumb,
),
(
"sample.ods",
ThumbRenderer._open_doc_thumb,
),
],
)
def test_document_preview(cwd, fixture_file, thumbnailer, snapshot):
file_path: Path = cwd / "fixtures" / fixture_file
img: Image.Image = thumbnailer(file_path)
img_bytes = io.BytesIO()
img.save(img_bytes, format="PNG")
img_bytes.seek(0)
assert img_bytes.read() == snapshot(extension_class=PNGImageSnapshotExtension)
And the error from each run (same output as if the original tests had missing snapshots):
E AssertionError: assert [+ received] == [- snapshot]
E Snapshot 'test_document_preview[sample.odt-_open_doc_thumb]' does not exist!
E + b'\x89PNG\r\n\x1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the test was renamed, it's looking for a fixture matching the new test name + parameters, which doesnt exist yet. You have to run pytest with --snapshot-update
the first time when a new test is created to generate the fixtures.
I've addressed the merge conflicts by combining some of the new and old tests using the Thank you for your review so far, and your help with the tests 👍 |
you can use
so it will look like this: from functools import partial
@pytest.mark.parametrize(
["fixture_file", "thumbnailer"],
[
(
"sample.pdf",
partial(ThumbRenderer._pdf_thumb, size=200),
), |
The question for future consideration is if all the thumbnailing methods shouldnt accept the same parameters (ie. |
This PR is a port of #366 which previously targeted thumbnails/Alpha-v9.4 to main (aka v9.5x).
This adds thumbnail/preview support for OpenDocument file types. This uses the code from #366 and allows any other OpenDocument file type to be rendered with the same method as MuseScore files.
Closes #379.