Skip to content
This repository has been archived by the owner on Apr 23, 2021. It is now read-only.

Tests sendwebpush #11

Closed
wants to merge 5 commits into from
Closed

Tests sendwebpush #11

wants to merge 5 commits into from

Conversation

gauntface
Copy link
Contributor

This adds methods for testing sendWebPush in it's current state.

@gauntface
Copy link
Contributor Author

cc @petele @wibblymat @addyosmani @jeffposnick

Anyone for a review?

@@ -0,0 +1,26 @@
/*
Copyright 2014 Google Inc. All Rights Reserved.

Choose a reason for hiding this comment

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

Might as well switch to 2016.

@jeffposnick
Copy link

Looks good in general. One piece of feedback with a scope that extends outside this PR is that it could be more maintainable to create a standalone file that included an enum-like mapping of error identifiers to error strings, and then refer to the errors via their identifier when constructing them in the library and when checking for them in the test.

@gauntface
Copy link
Contributor Author

Ta for the review Jeff (The mocha exit with status 0 is super frustrating!!).

Raised an issue regarding the error message (agreed that this doesn't scale well or maintain well) #12

@gauntface
Copy link
Contributor Author

@wibblymat when you get a chance, I'd appreciate you check over the code in push.js to make sure you are happy with them and I'm not missing anything.

@wibblymat
Copy link
Contributor

Superceded by #18

@wibblymat wibblymat closed this Apr 1, 2016
@wibblymat wibblymat deleted the tests-sendwebpush branch April 1, 2016 13:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants