test(scan): cover UTF-8 decoding of JSONL datasets#2550
Conversation
The dataset generator reads JSONL as UTF-8 (introduced in Giskard-AI#2544), but no test exercised non-ASCII content: the existing stubs use json.dumps with the default ensure_ascii=True, so they only ever contain ASCII. Add a regression test that writes a stub with non-ASCII scenario names (Latin-accented, CJK and Cyrillic) as UTF-8 and asserts they round-trip, locking in the UTF-8 read behaviour independently of the platform locale. Signed-off-by: xiaweiwei67-stack <xiaweiwei67@gmail.com>
![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 adds a new asynchronous test, 'test_dataset_generator_reads_utf8_content', to verify that non-ASCII scenario content is correctly decoded as UTF-8 regardless of the system locale. There are no review comments, and I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
kevinmessiaen
left a comment
kevinmessiaen
left a comment
There was a problem hiding this comment.
Thanks, the test is a good addition but I'd move the import at top of the file
Move json and base_mod imports out of the individual test functions to the module top, removing the per-test inline duplicates. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks for the review! I moved the imports to the top of the file, and hoisted the existing in-function imports there too for consistency. |
|
Thanks for applying the import hoist and merging main directly -- my local push raced with yours, so I've dropped my duplicate commit. Looks good on my end; happy to make any further tweaks if needed. |
2 participants
What
Add a regression test covering UTF-8 decoding of JSONL datasets in
DatasetScenarioGenerator.Why
#2544 changed the dataset loader to read JSONL with
encoding="utf-8", but no existing test exercises non-ASCII content: the stubs build their JSON withjson.dumps(...)(defaultensure_ascii=True), so they only ever contain ASCII and would pass even if the loader fell back to the platform locale encoding.Change
tests/generators/test_base.py: addtest_dataset_generator_reads_utf8_content, which writes a stub with non-ASCII scenario names (Latin-accented, CJK, Cyrillic) usingencoding="utf-8"and asserts they round-trip through the generator. This locks in the UTF-8 read behaviour independently of the platform locale (on Windows the defaultopen()codec is not UTF-8, so without #2544 this test would fail).Verification
pytest tests/generators/test_base.py-> 7 passedruff check tests/generators/test_base.py-> clean