-
-
Notifications
You must be signed in to change notification settings - Fork 12
Implement subscription filter option #9
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
base: master
Are you sure you want to change the base?
Conversation
| filter: function (path, message, options) { | ||
| return true | ||
| } | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a test where the filter function return false? Also, you'll need to add some assertions on path, message and options.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the filter function returns false the test times out b/c the message doesn't arrive. I'm not sure how to assert this. I will add assertions for three params as soon as I got some time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just create a different experiment where you do not call pubSubTest, but rather write actual tests for things. Also, it will require a test for promise form of the async function.
* Update to @hapi/nes 12.x and @hapi/hapi 19.x * Update dependencies and switch tests to @hapi/lab and @hapi/code * Update CI to Node 12 (Hapi requirement)
package.json
Outdated
| { | ||
| "name": "multines", | ||
| "version": "1.0.0", | ||
| "version": "2.0.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please restore this?
| { | ||
| "name": "multines", | ||
| "version": "1.0.0", | ||
| "version": "2.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems still a bump.
Here's the first try on an implementation. It works fine in my project but I need to test if the params and credentials arguments are properly working first. I've added these to stay as compatible with nes as possible. I'm not sure how to test the filter option properly though.
Submitting the pr so you can take a look and give feedback in case would like to do something different or have other suggestions.