-
Notifications
You must be signed in to change notification settings - Fork 763
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
Discussion: Error messages #1933
Comments
Overall I think this is a very good idea. In my mind one of the roles libraries play is to simplify the process for using a underlying tool, like Selenium. So something like translating maybe obscure error messages makes sense and really fit into the larger purpose or functionality I think our libraries should perform. I would be curious then to look at the next layer down which is around the technical specifics looking at ..
I would also like to see a side by side comparison of what we currently do and what the proposal would be. But overall I think this is a good idea which we should discuss more. |
Awesome that you like the (general) idea :) For the technical parts, there are some things I noticed from doing this in our code base. Only the message matters... to users. I think that there are 2 parts to this: Almost all uses of SeleniumLibrary will be in Robot rather than Python. For example, I write most keywords in Robot because very few people in the team can write Python. In Robot, an error is always a string so for all intents and purposes When you use SeleniumLibrary in Python, the error object can matter. I think that an error object that allows easy error message construction would be really nice. For example, I currently use this error constructor: Disclaimer: This code is 'good enough' for me, but probably not for here class CompanyNameRobotFailure(Exception):
short_message: str = None
url: str = None
long_message: str = None
error: str = None
def __init__(
self, short_message=None, long_message=None, page_url=None, error_message=None
) -> None:
if isinstance(error_message, CompanyNameRobotFailure):
self.short_message = error_message.short_message
self.long_message = error_message.long_message
self.url = error_message.url
self.error = error_message.error
else:
self.short_message = short_message
self.long_message = long_message
if page_url:
page_url = f"Page: '{page_url}'"
self.url = page_url
if isinstance(error_message, Exception):
error_message = str(error_message)
self.error = error_message
parts = [
self.short_message,
self.url,
self.long_message,
self.error,
]
parts = [p for p in parts if p is not None and p != ""]
super().__init__(" | ".join(parts)) raise CompanyNameRobotFailure(
short_message="Element is not visible",
long_message=f"Element '{locator}' not visible after {timeout_str}",
page_url=self._get_driver().current_url,
error_message=e,
) I imagine a Build the error message at the endRegardless of the keyword, you probably have all the information needed for a complete error message in the keyword function. There are some exceptions, I'll come back to those. With this idea, my keywords look something like this: *** Keywords ***
Our Custom Get Element attribute
[Arguments] ${element} ${attribute} ${timeout}=${TIMEOUT_DEFAULT}
Our Custom wait on element ${element} timeout=${timeout}
TRY
${value} = Wait Until Keyword Succeeds ${timeout} ${TIMEOUT_DEFAULT_RETRY_INTERVAL}
... GEN-private GUI Get Element attribute ${element} ${attribute}
RETURN ${value}
EXCEPT * NO_ATTRIBUTE type=GLOB
${location} = SeleniumLibrary.Get Location
${timeoutString} = Convert Time ${timeout} verbose
Our Custom Fail
... message=Element has missing attribute | Page: '${location}' | Element '${element}' has no attribute '${attribute}' after ${timeoutString}
... screenshotName=Get Element attribute
... screenshotHighlight=${element}
END
Private Our Custom Get Element attribute
[Documentation] Retryable part of GEN Wait and Get Element attribute
[Tags] robot:private robot:flatten
[Arguments] ${element} ${attribute}
${actualValue} = SeleniumLibrary.Get Dom Attribute ${element} ${attribute}
IF $actualValue == $None
Fail NO_ATTRIBUTE
END
RETURN ${actualValue} There is a lot of code here dedicated to making a meaningul error message. What I want to highlight here is that the error message is constructed in the keyword itself. The actual logic of what we're waiting on is separated to the secondary (private) keyword. If you look at the code, you may spot some features we're ignoring for this discussion. If you're interested in those, we can create a spin-off discussion ;) By building the error messages in the keyword itself, you can easily see which keywords use an extended error message without digging into the code. This also allows you to overwrite any error messages created deeper in the code, if you whish. However, this is a double edged sword. Because of this, I sometimes don't even touch the detailed error message. I may do something like: *** Keywords ***
Our Company Location Should Contain
[Arguments] ${partialUrl} ${timeout}=${TIMEOUT_DEFAULT}
TRY
SeleniumLibrary.Wait until Location contains
... expected=${partialUrl}
... timeout=${timeout}
EXCEPT AS ${e}
GEN GUI Fail
... message=Unexpected location | ${e}
... screenshotName=Location Should Contain
END Here, I simply pass along Your questionsI already attempted to cover your questions above, but for clarity sake:
It'll probably be a whole new exception. The messages inside that exception can come from multiple places.
I would replace most/all Selenium error messages. I would do this by writing the error messages in the top-level keyword functions.
All/most are translated. This is guaranteed by doing it in the top-level keyword functions and by writing tests that assert the expected error message.
I think it does not. This should be about the contents of error messages. I don't think it's worth changing the contents of error messages based on the log level. |
I've been using SeleniumLibrary for a while now. It's been great. I have come across issues and headaches, but almost all of them are caused by Selenium itself :)
Context
We're using SeleniumLibrary within a significantly sized project with 10+ testers. Some of these testers are not well-versed in code or technical things. Because of this situation, I find myself wrapping most SeleniumLibrary keywords for two reasons: adding retrying behavior and enhancing the error messages. I want to focus on the second reason for this issue.
Our error messages
To me, the error messages are a form of documentation. Documentation that pops up the exact moment you need to know about something. Because of this idea, I write error messages like this:
This error message has the following parts:
Element is not visible
Short description without details. This is useful for quickly scanning multiple fails, especially in our pipeline test report. In the pipeline report, we only have the test name and the fail message. The short message allows you to decide if you want more details. If you do, you keep reading.
Page: 'https://example.com/page'
On what page did the error occur? This is useful for debugging and recognizing known issues. I think known issues should not exist, but reality teaches us that they happen. Being able to recognize them is never a bad thing.
Element 'div.example.locator' not visible after 5 seconds
The actual details of what went wrong. This part can consist of multiple parts, depending on the keyword. The idea is to give as many details as possible. These details are used to figure out what exactly happened. Other examples from our codebase include:
Simple one:
Could not scroll element 'div.example.locator' into view
Multiple parts:
Element 'div.example.locator' contains unexpected text after 5 seconds | 'Foo' (expected) in 'Who is this Foo fella?' (actual)
Multiple parts with lots of details:
Element 'div.example.locator img' attribute 'alt' does not contain 'foo' after 5 seconds | 'Amazing image alt tag contents' does not contain 'foo'
Screenshot: 0-FAIL-meaningful-name.png
The file name of the screenshot. This allows you to easily reference a screenshot if it was made. If no screenshot is made, this part will be
Screenshot: [None]
.Examples
Same example as above
Lots of details
Note that the 3rd part is the exact error from
Wait until Location contains
Note that the 3rd part is the exact error from
Wait Until Element Is Enabled
Discussion topic
I'm a fan of solving something like this in a great way, only once, in the right place. I think the right place for these error messages is in SeleniumLibrary. I also think the existing error messages are pretty good but can be improved.
What do you think about adding (some of) these ideas to SeleniumLibrary?
Please note that I'm willing to help implement these changes.
The text was updated successfully, but these errors were encountered: