Remove ansi escape chars in assertExpectedInline and add options to skip comments and to skip empty lines #134248
Conversation
…optins to skip comments and empty lines [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/134248
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (1 Unrelated Failure)As of commit 888c7ee with merge base 938f37b ( BROKEN TRUNK - The following job failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
laithsakka
changed the title
laithsakka
changed the title
aorenste
left a comment
aorenste
left a comment
There was a problem hiding this comment.
Is this actually better than finding where the escape codes are coming from and adding an option to turn them off?
(Where are they coming from? Something internal or external?)
we can but why not making the tool insensitive to those?
|
yeh i think its better, since when comparing strings anyway those are not taken into account. |
aorenste
left a comment
aorenste
left a comment
There was a problem hiding this comment.
Approving - I think this isn't unreasonable. In general our tools shouldn't be unconditionally emitting ansi codes in their output, but maybe that's a separate battle.
| def assertExpectedInline(self, actual, expect, skip=0): | ||
| def _remove_ansi_escape(self, input): | ||
| # 7-bit C1 ANSI sequences | ||
| ansi_escape = re.compile(r''' |
There was a problem hiding this comment.
I dislike the fact that this regex uses sequences that cross chartype boundaries (like "@-Z") but in general is reasonable.
There was a problem hiding this comment.
alternatively I can add a flag ignore_ansi_escape, like the other two flags.
…ptions to skip comments and to skip empty lines" I had a night mare rewriting tests in test_misc.py specifically : 1. graphs can have comments that refers to my files "/lsakka/.." we really dont care about comments add option to ignore comments. 2. empty lines added when EXPECTTEST_ACCEPT=1 are changed with linter causing tests to fail or linter fail! add flag to ignore empty lines. 3. EXPECTTEST_ACCEPT fails when the text have some not readable characters. those should not effect comparing strings, also those causes weird diffs comments when tests fails. I removed ansi_escape chars #133045 this is used in [ghstack-poisoned]
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
4 participants
I had a night mare rewriting tests in test_misc.py specifically :
add flag to ignore empty lines.
this is used in
Stack from ghstack (oldest at bottom):