Skip to content

Testing the Renderer's Backwards Compatibility - #7

Closed
ilkerkesen wants to merge 1 commit into
sign:mainfrom
ilkerkesen:backwards-compatibility-tests
Closed

Testing the Renderer's Backwards Compatibility#7
ilkerkesen wants to merge 1 commit into
sign:mainfrom
ilkerkesen:backwards-compatibility-tests

Conversation

@ilkerkesen

Copy link
Copy Markdown
Contributor

This is related to #2, and it should fix the issue also as well. This PR implements backward compatibility tests for the renderer. There are 3 new files,

  • examples/pixel_renderer/samples.tsv contains example sentences from SIB-200. This file includes 5 examples per language, covering all 200 langauges present in the SIB-200 benchmark.
  • examples/pixel_renderer/create_sample_images.py: This script reads the samples.tsv (via --input-file option) and produces sample rendered text images per language at the specified --output-dir, using the provided set of example sentences. So, instead of saving all images individually, this script groups examples by language, concatenates visually rendered sentences vertically, and produces one image file per language (e.g. eng_Latn.png). It copies the samples.tsv file into this directory, and additionally creates a JSON file versions.json which contains the current versions of related packages (e.g. pango, pygobject).
  • tests/pixel_renderer/test_backward_compatibility.py: This is the main test file. It takes the sample image directory path via the environment variable SAMPLE_IMAGES_DIR. I don't know whether there exists a more viable solution to pass command line arguments using pytest.

ruff check does not create any warning/error for these new files. I used AI tools to implement tests where I reviewed the code accordingly in detail. I was not completely sure about where I should have put samples.tsv and create_sample_images.py files. We can also move these files to somewhere else. Also, if there is better way to pass the sample image dir value other than environment variable, let's follow that approach. Lastly, this PR is not ready for merge, because I need to write some documentation about it.

@@ -0,0 +1,128 @@
import csv

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this file contains lots of repeated code from examples/pixel_renderer/create_sample_images.py
I think you could move all three files under tests/pixel_renderer/regression
then you can import, in the test_ file the relevant functions.
there should also be tests/pixel_renderer/regression/assets with all of the sample images (you should push the images to the repo)

Taken from the SAMPLE_IMAGES_DIR environment variable.
If unset, all tests will be skipped.
"""
path = os.environ.get("SAMPLE_IMAGES_DIR")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

once the files are in the same dir, you can Path(__file__).parent / "assets" or something, then there's no env variable

@AmitMY

AmitMY commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Thanks a lot for this, @ilkerkesen! Since we couldn't push the review changes to your fork, we carried this work over to #15 (with you credited as co-author) — it applies the review feedback: everything moved under tests/pixel_renderer/regression/, the test imports the helpers from create_sample_images.py instead of duplicating them, the env variable is replaced with paths relative to the test file, and the reference images (generated on a CI-matching Ubuntu 24.04 environment) are committed to the repo. versions.json is now also checked by the tests, so a pango/cairo drift skips with a "regenerate references" message instead of failing everything. Closing this one in favor of #15.

@AmitMY AmitMY closed this Jul 14, 2026
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.

2 participants