Skip to content
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

Glossary Removal #1316

Closed

Conversation

willsaid-google
Copy link

@willsaid-google willsaid-google commented May 28, 2020

Description Glossary Removal from Earl Grey Error Handling:

Recently, glossary contents have not been used at all, although they sometimes contain needed information (such as, what was the picker and the column that failed?)

If the glossary contents were important to the user, i added them to the exception reason.
If they weren't, i removed the glossary contents.

I also removed the glossary properties on GREYError, and the user info dictionary property as well being passed into I_GREYErrorMake, and the GREYPopulateErrorNoted macro, all of which were complicating error handling and making errors harder to read.

Example 1, where important info was not being shown, before:

picker-glossary-before

after:

picker-glossary-after

Example 2
this time when the glossary contained too much info, before:

glossary-before

and after:

glossary-after

Of course, in this second example, a followup edit should remove the unnecessary Element Matcher key for this specific error code but that is outside the scope of the glossary removal.

Example 3 - Idling Resources
In this example, the output hasn't changed at all, although the glossary was removed.
idling

Example 4 - Stepper
If the stepper buttons aren't found, before:

Failed to find stepper buttons
in [Stepper]

Notice how it wasn't printing out the Stepper that is included in the glossary.
After:

Failed to find stepper buttons in stepper: 
<UIStepper: 0x7fdaaf52e030; frame = (205 35; 94 32); clipsToBounds = YES; opaque = NO; autoresize = RM+BM; layer = <CALayer: 0x6000026fa6a0>>

NSString *description = [NSString stringWithFormat:
@"Keyboard did not appear after tapping on an element."
@"Are you sure that tapping on this element will bring up the keyboard?"
@"\nElement: %@", element];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NSString *description = 
    [NSString stringWithFormat:@"Keyboard did not appear after tapping on an element."
                               @"Are you sure that tapping on this element will bring up the keyboard?"
                               @"\nElement: %@", element];

return NO;
}
}

// If a position is given, move the text cursor to that position.
__block id firstResponder = nil;
__block NSDictionary<NSString *, NSString *> *errorGlossary = nil;
__block NSMutableString *description = nil;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

__block NSMutableString<NSString *, NSString *>

NSString *description = [NSString stringWithFormat:@"Failed to find stepper buttons in stepper:"
@"%@", [stepper description]];
I_GREYPopulateError(error, kGREYInteractionErrorDomain,
kGREYInteractionActionFailedErrorCode, description);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

format

NSString *description = [NSString stringWithFormat:
@"Keyboard did not appear after tapping on an element."
@"Are you sure that tapping on this element will bring up the keyboard?"
@"\nElement: %@", element];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of newline for each items of the glossary, how about adding numbering, or bullet points? I feel like that would be easier and cleaner than adding spaces, as well as parsing (if we have to). What do you think?

[reason appendFormat:@"Assertion with [Matcher] failed: UI [Element] failed to match "
@"the following matcher(s): [Mismatch]"];
glossary[@"Matcher"] = [matcher description];
glossary[@"Element"] = [element grey_description];
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add these back in to the reason

@tirodkar tirodkar closed this Jun 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants