Fix validation plugin converting characters to Unicode escape sequences (CMEM-8068) - #6
Merged
Merged
Conversation
json.dumps() defaults to ensure_ascii=True, which escaped non-ASCII characters (e.g. ö) into unicode escape sequences when valid JSON objects were written to the target JSON dataset. Also fixed test_source_and_target_dataset, which exercises this exact code path: it was never collected by pytest because its name didn't start with test_ (validate_test_... instead of test_...) and it had no @needs_cmem marker, unlike its siblings.
Coverage Report
|
A fully mocked Client can't exercise the real upload path, so it wouldn't have caught issues like the io.StringIO/io.BytesIO mismatch just found in cmem-plugin-graphql's equivalent code. Use a dedicated non-ASCII fixture dataset and the existing source-dataset test flow instead, checking the raw target file bytes for escape sequences.
Corrects fail_on_violations descriptions that claimed early termination on the first violation, when both tasks actually validate everything before failing the report. Fills in missing descriptions for boolean parameters in validate_graph, fixes grammar bugs in the validate_entities choice labels, and rewrites the Input/Output Modes sections to describe port shape instead of restating the parameters.
msaipraneeth
added a commit
that referenced
this pull request
Sep 8, 2026
Merging PR #6 interleaved its Unreleased entries into the already-tagged [1.3.1] section instead of creating a new [Unreleased] heading, because the branch diverged before 1.3.1 was cut and git's line-based merge couldn't tell the two apart. Splits them back into a proper [Unreleased] section ahead of the published 1.3.1 entry.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
json.dumps()defaults toensure_ascii=True, which silently escaped non-ASCII characters (e.g.ö) into\uXXXXsequences when the Validate Entities task wrote valid JSON objects to the target JSON dataset.ensure_ascii=Falseso the JSON representation of the original content is preserved.test_source_and_target_dataset, which exercises this exact code path: it was never collected by pytest because its name didn't start withtest_(validate_test_...instead oftest_...), and it had no@needs_cmemmarker unlike its siblings.Validate EntitiesandValidate Knowledge Graphplugin documentation and parameter descriptions (e.g.fail_on_violationsclaimed the task fails on the first violation, when it actually validates everything before failing the report; several boolean parameters invalidate_graphhad no description at all).Test plan
test_target_dataset_keeps_unicode_characters, an isolated unit test using a fakeClient(viamonkeypatch) so it runs without a Corporate Memory connection; confirmed it fails against the pre-fix code with the exactöescape, then passes with the fix.ruff checkandmypy -p tests -p cmem_plugin_validation(package + tests) pass.pytestrun: all tests that don't require a live CMEM connection pass.