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

Debug logs #178

Merged
merged 5 commits into from
Apr 24, 2018
Merged

Debug logs #178

merged 5 commits into from
Apr 24, 2018

Conversation

damencho
Copy link
Member

@Override
public List getBrowserLogs()
{
return (List)executeScript("return APP.debugLogs.getLogs();");
Copy link
Member

Choose a reason for hiding this comment

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

can we try catch RuntimeException here and return empty List or null in case "APP" will be undefined ?

Copy link
Member

Choose a reason for hiding this comment

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

also there's missing space after (List)

Copy link
Member Author

Choose a reason for hiding this comment

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

This is fixed.


return driver.manage().logs().get(LogType.BROWSER);
}
public abstract List getBrowserLogs();
Copy link
Member

Choose a reason for hiding this comment

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

can we specify what kind of list that is ? List or whatever

Copy link
Member

Choose a reason for hiding this comment

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

sorry, I meant List<Object>

Copy link
Member Author

Choose a reason for hiding this comment

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

What is the point of having that?

Copy link
Member

Choose a reason for hiding this comment

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

Because in one doc you write it's a string in other there can be just anything and it's not obvious what's there. It's just kind of self documenting, but you can leave it as is if you don't like that

Copy link
Member Author

Choose a reason for hiding this comment

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

I just don't see the point of having that. If there is nothing it is Object.

Copy link
Member

Choose a reason for hiding this comment

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

yeah, maybe I just got used to templates somehow

{
return (List) executeScript("return APP.debugLogs.getLogs();");
}
catch (Throwable t)
Copy link
Member

Choose a reason for hiding this comment

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

not Throwable, Exception at best, but ideally a RuntimeException

@paweldomas paweldomas merged commit 1afa4c5 into master Apr 24, 2018
@damencho damencho deleted the debug-logs branch May 7, 2018 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants