crossbario / autobahn-js

WAMP in JavaScript for Browsers and NodeJS
http://crossbar.io/autobahn
MIT License
1.43k stars 228 forks source link

Order of event delivery with multiple matching subscriptions #408

Open oberstet opened 5 years ago

oberstet commented 5 years ago

Just reran our tests, and this one differs from the checked in result:

(cpy372_5) oberstet@intel-nuci7:~/scm/crossbario/autobahn-js$ git diff test/
diff --git a/test/test_pubsub_multiple_matching_subs.txt b/test/test_pubsub_multiple_matching_subs.txt
index 0836bd5..74f505a 100644
--- a/test/test_pubsub_multiple_matching_subs.txt
+++ b/test/test_pubsub_multiple_matching_subs.txt
@@ -1,12 +1,12 @@
 0 "all sessions connected"
 1 "event: args=" ["hello subscriber!",1] ", kwargs=" {}
-2 "subscription: sub_topic=" "com..topic1" ", sub_match=" "wildcard" ", session_ident=" "B"
+2 "subscription: sub_topic=" "com.myapp.topic1" ", sub_match=" "exact" ", session_ident=" "E"
 3 "event: args=" ["hello subscriber!",1] ", kwargs=" {}
-4 "subscription: sub_topic=" "com.myapp.topic1" ", sub_match=" "exact" ", session_ident=" "E"
+4 "subscription: sub_topic=" "com" ", sub_match=" "prefix" ", session_ident=" "D"
 5 "event: args=" ["hello subscriber!",1] ", kwargs=" {}
-6 "subscription: sub_topic=" "com" ", sub_match=" "prefix" ", session_ident=" "D"
+6 "subscription: sub_topic=" "com.myapp" ", sub_match=" "prefix" ", session_ident=" "C"
 7 "event: args=" ["hello subscriber!",1] ", kwargs=" {}
-8 "subscription: sub_topic=" "com.myapp" ", sub_match=" "prefix" ", session_ident=" "C"
+8 "subscription: sub_topic=" "com..topic1" ", sub_match=" "wildcard" ", session_ident=" "B"
 9 "event: args=" ["hello subscriber!",2] ", kwargs=" {}
 10 "subscription: sub_topic=" "com" ", sub_match=" "prefix" ", session_ident=" "D"
 11 "event: args=" ["hello subscriber!",2] ", kwargs=" {}

This is not a bug!

The reason for the diff is:

oberstet commented 5 years ago

Ultimately, we need to discuss this on the WAMP protocol repo - and also probably come up with a better unit test approach here in ABJS (and also CI for that), but that is orthogonal to the former

om26er commented 5 years ago

Similar to https://github.com/crossbario/autobahn-js/issues/383 -- Do you think it will make sense to disable that test for now ? Because if we are to enable CI for this repo, we need our tests to always pass.

oberstet commented 5 years ago

Do you think it will make sense to disable that test for now ?

yea, that might be necessary until we come up with a real solution, and I'd be cool with deactivating the test until then .. we should file an issue specifically for the deactivated test to track it and not forget