-
Notifications
You must be signed in to change notification settings - Fork 42
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
chore(filter v2): test updates #811
Conversation
- test incorrect subscribe identifier separated
- test contentTopics limit
Jenkins BuildsClick to see older builds (124)
|
…t-updates # Conflicts: # waku/v2/protocol/filter/client.go # waku/v2/protocol/filter/options.go
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.
Left some nits/suggestions
Combine log messages Co-authored-by: richΛrd <info@richardramos.me>
Delete commented - temporary code Co-authored-by: richΛrd <info@richardramos.me>
fmt.Sprintf instead of "+" suffix => more performance and beauty Co-authored-by: richΛrd <info@richardramos.me>
Adjust comment with code Co-authored-by: richΛrd <info@richardramos.me>
Combine multiple related log entries into one. Co-authored-by: richΛrd <info@richardramos.me>
Align comment with the code Co-authored-by: richΛrd <info@richardramos.me>
Use fmt.Sprintf() instead of "+" for more beauty and speed Co-authored-by: richΛrd <info@richardramos.me>
…to chore(filterV2)-test-updates
@romanzac , Was going through the failed test go-waku/waku/v2/protocol/filter/client.go Line 169 in 9161c4f
So even if we add a wrong protocol to the client, that would just be an additional protocol supported by this lightClient and does not affect how it communicates with filter node. |
@richard-ramos , for some reason Test for WakuMetadata proto seems to also fail. |
Fixed in #826 |
@chaitanyaprem Filter tests are passing on my side. I have followed your suggestions for "TestIncorrectSubscribeIdentifier". It is working, although I had to go deep into the stack and copied a lot of code. Not sure, if by chance you would find beneficial to have subscribe protocol identifier changeable at higher levels. Or perhaps Peerstore().RemoveProtocols() might work ? |
Actually RemoveProtocols might have worked, slipped my mind. Also, do note that this is a kind of test you are trying to do which may sometimes require emulating lot of client code(as you are trying to simulate a scenario which would not be possible with existing client implementations) |
Also looks like #836 seems to have slipped past your tests and mine. Haven't reviewed all your tests though, so not sure if it is already covered. :) |
- test preview Subscribe fullNode to fullNode
Do you mean in general, something is called with initialized variable, but no specific values set ? I will come to these later. |
I will reuse those "copied" funcs in later tests where I go down to the level of invalid payloads. BTW: I have tried RemoveProtocols and it didn't work for me. I see from the code subscribe related protocol identifier is not part of a live node instance. It is part of the code only. Could you confirm for me please ? |
Guys, I wish we can close this PR by Friday this week. Its scope is too large. I will create smaller PRs starting next week. |
Oh right, since the protocol stream is created only when a request has to be sent which means anytime call to subscribe API will initiate a new stream with the protocol and hence not possible. |
May I proceed with merge to master ? |
:( We shouldn't merge PRs that have not been approved |
--------- Co-authored-by: Richard Ramos <info@richardramos.me> Co-authored-by: Prem Chaitanya Prathi <chaitanyaprem@gmail.com>
I am sorry, I've interpreted silence for one day as yes. I've announced I would like to close it by Friday and approval was not hardwired as precondition to merge. I will ping you on Discord next time. |
* Chore(filter v2) test updates (#811) * test: Test incorrect protocol identifiers * fix: return errors in FilterSubscribeOption * test: Test incorrect push identifier added - test incorrect subscribe identifier separated * test: Test Ping failure after unsubscription * test: Test PubSub with single content topic * test: Simplify test PubSub with single content topic * test: Test with single pubsub and multiple content topics * test: Test with multiple PubSub and multiple contentTopic * test: Test with multiple overlaping contentTopics - test contentTopics limit * test: refactor tests to fix concurrent run errors * test: Test subscription refresh * test: Test error handling for subscribe * test: Test subscription to multiple full nodes * update test to fix #804 * Update waku/v2/protocol/filter/filter_test.go Combine log messages Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Delete commented - temporary code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go fmt.Sprintf instead of "+" suffix => more performance and beauty Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Adjust comment with code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Combine multiple related log entries into one. Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Align comment with the code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Use fmt.Sprintf() instead of "+" for more beauty and speed Co-authored-by: richΛrd <info@richardramos.me> * test: refactor tests with prepareData() * test: Test incorrect protocol identifiers * chore: rebase onto latest master * test: Test incorrect push identifier added - test incorrect subscribe identifier separated * test: Test Ping failure after unsubscription * test: Test PubSub with single content topic * test: Simplify test PubSub with single content topic * test: Test with single pubsub and multiple content topics * test: Test with multiple PubSub and multiple contentTopic * test: Test with multiple overlaping contentTopics - test contentTopics limit * test: refactor tests to fix concurrent run errors * test: Test subscription refresh * test: Test error handling for subscribe * test: Test subscription to multiple full nodes * update test to fix #804 * Update waku/v2/protocol/filter/filter_test.go Combine log messages Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Delete commented - temporary code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go fmt.Sprintf instead of "+" suffix => more performance and beauty Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Adjust comment with code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Combine multiple related log entries into one. Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Align comment with the code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Use fmt.Sprintf() instead of "+" for more beauty and speed Co-authored-by: richΛrd <info@richardramos.me> * test: refactor tests with prepareData() * test: Test incorrect protocol identifiers * fix: return errors in FilterSubscribeOption * test: Test incorrect push identifier added - test incorrect subscribe identifier separated * test: Test Ping failure after unsubscription * test: Test PubSub with single content topic * test: Simplify test PubSub with single content topic * test: Test with single pubsub and multiple content topics * test: Test with multiple PubSub and multiple contentTopic * test: Test with multiple overlaping contentTopics - test contentTopics limit * test: refactor tests to fix concurrent run errors * test: Test subscription refresh * test: Test error handling for subscribe * test: Test subscription to multiple full nodes * update test to fix #804 * Update waku/v2/protocol/filter/filter_test.go Combine log messages Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Delete commented - temporary code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go fmt.Sprintf instead of "+" suffix => more performance and beauty Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Adjust comment with code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Combine multiple related log entries into one. Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Align comment with the code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Use fmt.Sprintf() instead of "+" for more beauty and speed Co-authored-by: richΛrd <info@richardramos.me> * test: refactor tests with prepareData() * Fix error during rebase * Sync filter tests with latest master * Refactor context initialization for test * test: Incorrect Subscribe Identifier refactored with custom subscribe * test: refactor into multiple files * test: Subscribe with multiple light nodes to one full node * test: shared mode for full node creation - test preview Subscribe fullNode to fullNode * test: test Subscribe fullNode to fullNode --------- Co-authored-by: Richard Ramos <info@richardramos.me> Co-authored-by: Prem Chaitanya Prathi <chaitanyaprem@gmail.com> * test: unsubscribe with single contentTopic * test: extend test - unsubscribe with single contentTopic * test: unsubscribe with multiple contentTopic * test: unsubscribe with multiple pubSub/contentTopic * test: refactor back to use waitForTimeout() * test: unsubscribe error handling --------- Co-authored-by: Richard Ramos <info@richardramos.me> Co-authored-by: Prem Chaitanya Prathi <chaitanyaprem@gmail.com>
* Chore(filter v2) test updates (#811) * test: Test incorrect protocol identifiers * fix: return errors in FilterSubscribeOption * test: Test incorrect push identifier added - test incorrect subscribe identifier separated * test: Test Ping failure after unsubscription * test: Test PubSub with single content topic * test: Simplify test PubSub with single content topic * test: Test with single pubsub and multiple content topics * test: Test with multiple PubSub and multiple contentTopic * test: Test with multiple overlaping contentTopics - test contentTopics limit * test: refactor tests to fix concurrent run errors * test: Test subscription refresh * test: Test error handling for subscribe * test: Test subscription to multiple full nodes * update test to fix #804 * Update waku/v2/protocol/filter/filter_test.go Combine log messages Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Delete commented - temporary code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go fmt.Sprintf instead of "+" suffix => more performance and beauty Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Adjust comment with code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Combine multiple related log entries into one. Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Align comment with the code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Use fmt.Sprintf() instead of "+" for more beauty and speed Co-authored-by: richΛrd <info@richardramos.me> * test: refactor tests with prepareData() * test: Test incorrect protocol identifiers * chore: rebase onto latest master * test: Test incorrect push identifier added - test incorrect subscribe identifier separated * test: Test Ping failure after unsubscription * test: Test PubSub with single content topic * test: Simplify test PubSub with single content topic * test: Test with single pubsub and multiple content topics * test: Test with multiple PubSub and multiple contentTopic * test: Test with multiple overlaping contentTopics - test contentTopics limit * test: refactor tests to fix concurrent run errors * test: Test subscription refresh * test: Test error handling for subscribe * test: Test subscription to multiple full nodes * update test to fix #804 * Update waku/v2/protocol/filter/filter_test.go Combine log messages Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Delete commented - temporary code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go fmt.Sprintf instead of "+" suffix => more performance and beauty Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Adjust comment with code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Combine multiple related log entries into one. Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Align comment with the code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Use fmt.Sprintf() instead of "+" for more beauty and speed Co-authored-by: richΛrd <info@richardramos.me> * test: refactor tests with prepareData() * test: Test incorrect protocol identifiers * fix: return errors in FilterSubscribeOption * test: Test incorrect push identifier added - test incorrect subscribe identifier separated * test: Test Ping failure after unsubscription * test: Test PubSub with single content topic * test: Simplify test PubSub with single content topic * test: Test with single pubsub and multiple content topics * test: Test with multiple PubSub and multiple contentTopic * test: Test with multiple overlaping contentTopics - test contentTopics limit * test: refactor tests to fix concurrent run errors * test: Test subscription refresh * test: Test error handling for subscribe * test: Test subscription to multiple full nodes * update test to fix #804 * Update waku/v2/protocol/filter/filter_test.go Combine log messages Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Delete commented - temporary code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go fmt.Sprintf instead of "+" suffix => more performance and beauty Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Adjust comment with code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Combine multiple related log entries into one. Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Align comment with the code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Use fmt.Sprintf() instead of "+" for more beauty and speed Co-authored-by: richΛrd <info@richardramos.me> * test: refactor tests with prepareData() * Fix error during rebase * Sync filter tests with latest master * Refactor context initialization for test * test: Incorrect Subscribe Identifier refactored with custom subscribe * test: refactor into multiple files * test: Subscribe with multiple light nodes to one full node * test: shared mode for full node creation - test preview Subscribe fullNode to fullNode * test: test Subscribe fullNode to fullNode --------- Co-authored-by: Richard Ramos <info@richardramos.me> Co-authored-by: Prem Chaitanya Prathi <chaitanyaprem@gmail.com> * test: unsubscribe all without content topics * test: unsubscribe all without any filter specification * test: move unsubscribe all tests to unsubscribe file --------- Co-authored-by: Richard Ramos <info@richardramos.me> Co-authored-by: Prem Chaitanya Prathi <chaitanyaprem@gmail.com>
* Chore(filter v2) test updates (#811) * test: Test incorrect protocol identifiers * fix: return errors in FilterSubscribeOption * test: Test incorrect push identifier added - test incorrect subscribe identifier separated * test: Test Ping failure after unsubscription * test: Test PubSub with single content topic * test: Simplify test PubSub with single content topic * test: Test with single pubsub and multiple content topics * test: Test with multiple PubSub and multiple contentTopic * test: Test with multiple overlaping contentTopics - test contentTopics limit * test: refactor tests to fix concurrent run errors * test: Test subscription refresh * test: Test error handling for subscribe * test: Test subscription to multiple full nodes * update test to fix #804 * Update waku/v2/protocol/filter/filter_test.go Combine log messages Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Delete commented - temporary code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go fmt.Sprintf instead of "+" suffix => more performance and beauty Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Adjust comment with code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Combine multiple related log entries into one. Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Align comment with the code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Use fmt.Sprintf() instead of "+" for more beauty and speed Co-authored-by: richΛrd <info@richardramos.me> * test: refactor tests with prepareData() * test: Test incorrect protocol identifiers * chore: rebase onto latest master * test: Test incorrect push identifier added - test incorrect subscribe identifier separated * test: Test Ping failure after unsubscription * test: Test PubSub with single content topic * test: Simplify test PubSub with single content topic * test: Test with single pubsub and multiple content topics * test: Test with multiple PubSub and multiple contentTopic * test: Test with multiple overlaping contentTopics - test contentTopics limit * test: refactor tests to fix concurrent run errors * test: Test subscription refresh * test: Test error handling for subscribe * test: Test subscription to multiple full nodes * update test to fix #804 * Update waku/v2/protocol/filter/filter_test.go Combine log messages Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Delete commented - temporary code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go fmt.Sprintf instead of "+" suffix => more performance and beauty Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Adjust comment with code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Combine multiple related log entries into one. Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Align comment with the code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Use fmt.Sprintf() instead of "+" for more beauty and speed Co-authored-by: richΛrd <info@richardramos.me> * test: refactor tests with prepareData() * test: Test incorrect protocol identifiers * fix: return errors in FilterSubscribeOption * test: Test incorrect push identifier added - test incorrect subscribe identifier separated * test: Test Ping failure after unsubscription * test: Test PubSub with single content topic * test: Simplify test PubSub with single content topic * test: Test with single pubsub and multiple content topics * test: Test with multiple PubSub and multiple contentTopic * test: Test with multiple overlaping contentTopics - test contentTopics limit * test: refactor tests to fix concurrent run errors * test: Test subscription refresh * test: Test error handling for subscribe * test: Test subscription to multiple full nodes * update test to fix #804 * Update waku/v2/protocol/filter/filter_test.go Combine log messages Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Delete commented - temporary code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go fmt.Sprintf instead of "+" suffix => more performance and beauty Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Adjust comment with code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Combine multiple related log entries into one. Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Align comment with the code Co-authored-by: richΛrd <info@richardramos.me> * Update waku/v2/protocol/filter/filter_test.go Use fmt.Sprintf() instead of "+" for more beauty and speed Co-authored-by: richΛrd <info@richardramos.me> * test: refactor tests with prepareData() * Fix error during rebase * Sync filter tests with latest master * Refactor context initialization for test * test: Incorrect Subscribe Identifier refactored with custom subscribe * test: refactor into multiple files * test: Subscribe with multiple light nodes to one full node * test: shared mode for full node creation - test preview Subscribe fullNode to fullNode * test: test Subscribe fullNode to fullNode --------- Co-authored-by: Richard Ramos <info@richardramos.me> Co-authored-by: Prem Chaitanya Prathi <chaitanyaprem@gmail.com> * string generators for testing * fix CodeQL findings * merge variants of UTF8 String generator into one --------- Co-authored-by: Richard Ramos <info@richardramos.me> Co-authored-by: Prem Chaitanya Prathi <chaitanyaprem@gmail.com>
Description
Batch of new tests for Waku Filter v2.
Changes
Tests available to run:
cd go-waku/waku/v2/protocol/filter
go test
Two tests failing at the moment - corresponding issues were opened.