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

Add console timing manual tests #11584

Merged
merged 2 commits into from
Jul 4, 2018

Conversation

domfarolino
Copy link
Member

Adds console timing manual tests. This, as well as https://github.com/web-platform-tests/wpt/blob/master/console/console-label-conversion.any.js should be enough to cover whatwg/console#138.

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

LGTM, although you could improve clarity for reviewers by adding newlines in the HTML source to group the expected output in the same way that they are grouped in the JS.

console.time({toString() {return "default"}});
console.timeLog({toString() {return "default"}});
console.timeLog({toString() {return "default"}}, "extra data");
console.timeEnd({toString() {return "default"}});
Copy link
Member

Choose a reason for hiding this comment

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

It'd be good to test toString() conversion with a non-default label as well, to guard against implementations that fall back to "default" if typeof !== string

console.timeLog("a label", "extra data");
console.timeEnd("a label");

console.timeEnd("b"); // should produce a warning
Copy link
Member

Choose a reason for hiding this comment

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

Same for timeLog?

@domfarolino domfarolino merged commit eff6365 into master Jul 4, 2018
@domfarolino domfarolino deleted the domfarolino/console-timing-manual-tests branch July 4, 2018 04:16
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.

3 participants