Repository navigation
FIX: Show an explicit empty converted value in the HTML scenario report - #3771
Open
KVR Vishnu (VISHNU0906) wants to merge 1 commit into
Open
KVR Vishnu (VISHNU0906) wants to merge 1 commit into
KVR Vishnu (VISHNU0906) wants to merge 1 commit into
Conversation
The HTML report picked the shown text with 'converted_value or original_value', so an intentionally empty conversion fell back to the original text and the report showed something that was never sent. Fall back to the original only when the converted value is missing or None, and add tests for empty text, empty media and the fallback. Fixes microsoft#3741 Signed-off-by: KVR Vishnu <46535652+VISHNU0906@users.noreply.github.com>
Author
|
@microsoft-github-policy-service agree |
This branch has not been deployed
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
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.
Description
Fixes #3741.
The HTML scenario report chose what to show with
converted_value or original_value. When a converter intentionally produces an empty string, that check treats it as missing and shows the original text instead, so the report shows something that was never sent to the target. The same pattern was used for media pieces.This change picks the shown value once per piece and only falls back to the original value when the converted value is missing or None. An empty string is now shown as empty, in both the text and media branches. Nothing else in the template changes.
cc Roman Lutz (@romanlutz) (issue author) and Justin Song (@jsong468) (added the HTML printer). Happy to adjust anything.
I used an AI assistant to help find and write this change. I read every changed line and ran the tests myself.
Tests and Documentation
Added three tests in
tests/unit/output/scenario_result/test_html.py:[image_path]with no original pathconverted_valueis None or missingThe two empty-value tests fail on main and pass with the fix. Locally:
pytest tests/unit/output: 395 passedpytest -n 4 --dist=loadfile tests/unit): 32112 passed, 476 skippedruff checkandruff format --checkclean on the changed filesNo documentation changes needed, and no notebooks were touched.