fix: restore legacy t2i template variable#7819
fix: restore legacy t2i template variable#7819bugkeep wants to merge 1 commit intoAstrBotDevs:masterfrom
Conversation
dosubot
Bot
added
size:XS
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the NetworkRenderStrategy to include the raw text field in the template data and adds a new unit test to verify this behavior. The review feedback suggests isolating the unit test from the filesystem by mocking the strategy's initialization and expanding the test coverage to include non-ASCII characters to ensure the UTF-8 encoding logic is correctly verified.
| fake_render_custom_template, | ||
| ) | ||
|
|
||
| strategy = NetworkRenderStrategy(base_url="https://example.com") |
There was a problem hiding this comment.
The instantiation of NetworkRenderStrategy triggers the TemplateManager constructor, which performs filesystem operations (creating directories and copying files) in the data directory. Unit tests should ideally be isolated from the filesystem to ensure they are fast, reproducible, and don't leave side effects in the environment. Consider mocking TemplateManager or the NetworkRenderStrategy.__init__ method to prevent these side effects.
| await strategy.render("hello", return_url=True, template_name="base") | ||
|
|
||
| assert captured["tmpl_str"] == "<html>{{ text }}</html>" | ||
| assert captured["tmpl_data"]["text"] == "hello" | ||
| assert captured["tmpl_data"]["text_base64"] == base64.b64encode( | ||
| b"hello" | ||
| ).decode("ascii") |
There was a problem hiding this comment.
The test currently uses a simple ASCII string ("hello"). Since the implementation explicitly handles UTF-8 encoding before base64 conversion, it is recommended to include non-ASCII characters (e.g., Chinese characters or emojis) in the test case to verify that the encoding logic works correctly for all supported inputs.
| await strategy.render("hello", return_url=True, template_name="base") | |
| assert captured["tmpl_str"] == "<html>{{ text }}</html>" | |
| assert captured["tmpl_data"]["text"] == "hello" | |
| assert captured["tmpl_data"]["text_base64"] == base64.b64encode( | |
| b"hello" | |
| ).decode("ascii") | |
| await strategy.render("hello 你好", return_url=True, template_name="base") | |
| assert captured["tmpl_str"] == "<html>{{ text }}</html>" | |
| assert captured["tmpl_data"]["text"] == "hello 你好" | |
| assert captured["tmpl_data"]["text_base64"] == base64.b64encode( | |
| "hello 你好".encode("utf-8") | |
| ).decode("ascii") |
References
- New functionality should be accompanied by corresponding unit tests to ensure correctness across different input types, such as non-ASCII characters.
|
Closing this as a duplicate of #7789 , which has already been merged. Thank you anyway for your contribution to the open-source community! |
2 participants
Fixes #7750.
Summary by Sourcery
Restore support for the legacy text template variable in the T2I network render strategy while preserving the new base64 variant.
Bug Fixes:
Tests: