Assertion string tweaks#148
Conversation
provide error message for what line dosen't match when comparing multiple str outputs
Also add assertion tests for diff
…sted frm cli.py LOL
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #148 +/- ##
==========================================
- Coverage 96.68% 96.20% -0.48%
==========================================
Files 26 26
Lines 1476 1530 +54
==========================================
+ Hits 1427 1472 +45
- Misses 49 58 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
How about you don't skill issue it before requesting a review? |
I already made another skill issue lol DAMIT |
…tal output is less than expected output
| matcher = difflib.SequenceMatcher(None, expected, actual) | ||
|
|
||
| diffLog = [] | ||
| # maxChars = min(Assertions.DIFF_MAX_CHARACTERS, len(actual), len(expected)) |
| if visibleCount >= maxChars: | ||
| break | ||
| diffLog.append(f"{RESET_COLOR}{GREEN_BG}{ch}{RESET_COLOR}") | ||
| visibleCount += 1 |
There was a problem hiding this comment.
You are repeating this block a lot, break it out into a helper function
| if visibleCount >= maxChars: | ||
| break | ||
|
|
||
| if tag == 'equal': |
There was a problem hiding this comment.
i think i might prefer pattern matching here - also - are there constants defined for these tags inside the diff tools lib?
| return actual | ||
|
|
||
| def _assertIterableEqual(self, expected, actual, msg: Optional[str] = None): | ||
| errorMsg = msg if msg else None |
There was a problem hiding this comment.
this is doing nothing - msg will already be none if its not defined.
| if expected[i] != actual[i]: | ||
| self._raiseFailure("output", expected[i], actual[i], msg) | ||
| if isinstance(expected[i], str): | ||
| errorMsg = f"Expected output line {i+1} does not match your output line {i+1}" |
There was a problem hiding this comment.
This will only display the last error message. If we do something like this were we are asserting in a loop, we prolly want to make it display a list of all the lines that dont match
| match = re.search(r"Diff Log output: (.*)$", actualMsg, re.DOTALL) | ||
| self.assertIsNotNone(match) | ||
|
|
||
| diff_log = re.sub(r"\x1b\[[0-9;]*m", "", match.group(1)) |
There was a problem hiding this comment.
for everyone's sanity, you need to either explain this regex in a comment, or assert in a different way.
There was a problem hiding this comment.
You are missing a bunch of tests. Please add tests for the msg changes that you made. Please add tests covering delete and insert cases for the diff stuff.
| errorMsg = f"Incorrect {shortDescription}.\n" + \ | ||
| f"Expected {shortDescription}: {expectedObject}\n" + \ | ||
| f"Your {shortDescription} : {actualObject}" | ||
| errorMsg = f"Incorrect {shortDescription}.\n" |
There was a problem hiding this comment.
i almost want to make the diff output configurable. IE you set a flag in the assertions class to enable it or not.
| f"Expected {shortDescription}: {expectedObject}\n" + \ | ||
| f"Your {shortDescription} : {actualObject}" | ||
| errorMsg = f"Incorrect {shortDescription}.\n" | ||
| if expectedObject is not None and actualObject is not None and isinstance(expectedObject, str) and isinstance(actualObject, str): |
There was a problem hiding this comment.
there are currently no tests covering this if statement. You need to verify that we dont attempt to use the diff stuff when we arent comparing string to string.
| if msg: | ||
| errorMsg += "\n\n" + str(msg) |
There was a problem hiding this comment.
How does this interact with the message changes you made later?
3 participants
Adds diff log highlighting between expected output and current output.
Highlights with green if a character is correct and red if a character is incorrect.\
@gregbell26 let me know if there is anything missing.