-
Notifications
You must be signed in to change notification settings - Fork 35
Improve Error Message Readability in Lightning Protocol Tests #140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,7 +20,27 @@ def path_to_str(self) -> str: | |
| result += f"{event}," | ||
| return f"{result}]" | ||
|
|
||
| @staticmethod | ||
| def decode_hex_data(hex_data: str) -> str: | ||
| """Decode hex data into readable text if possible""" | ||
| try: | ||
| # Try to decode as ASCII/UTF-8 | ||
| decoded = bytes.fromhex(hex_data).decode("utf-8", errors="replace") | ||
| # If the decoded text is mostly readable, return it | ||
| if all(ord(c) < 128 for c in decoded): | ||
| return f" (decoded: {decoded})" | ||
| except (ValueError, UnicodeDecodeError): | ||
| pass | ||
| return "" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not understanding whe need to return an empty string here, can you explainit to me? |
||
|
|
||
| def __str__(self) -> str: | ||
| # Look for hex data in the message and try to decode it | ||
| parts = self.message.split("data=") | ||
| if len(parts) > 1: | ||
| hex_data = parts[1].split()[0] # Get the hex data before any spaces | ||
| decoded = self.decode_hex_data(hex_data) | ||
| if decoded: | ||
| self.message = self.message.replace(hex_data, f"{hex_data}{decoded}") | ||
| return f"`{self.message}` on event {self.path_to_str()}" | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -374,8 +374,11 @@ def action(self, runner: "Runner") -> bool: | |
|
|
||
| err = self.message_match(runner, msg) | ||
| if err: | ||
| raise EventError(self, "{}: message was {}".format(err, msg.to_str())) | ||
|
|
||
| # Format the error message to be more readable | ||
| error_msg = f"Expected {self.msgtype}, got {msg.messagetype.name}" | ||
| if hasattr(msg, "fields"): | ||
| error_msg += f": {msg.to_str()}" | ||
|
Comment on lines
+379
to
+380
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do not understand why we are checking the |
||
| raise EventError(self, error_msg) | ||
| break | ||
| return True | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really dangerous, please do not try to shoot yourself in the foot when you do not need it, we are already using Python.