-
-
Notifications
You must be signed in to change notification settings - Fork 122
Use '+' as single level wildcard to make it consistent with mqtt ascoltatore #62
Conversation
Could you please add a test for it? |
Add it in the behave_as_ascoltatore. As the MemoryAscoltatori cannot be made to pass these easily, feel free to drop it entirely: trie rocks. |
Should be done with that commit, was more work than I thought it would be. |
The build is failing :( |
Strange - builds here. I'll look into it, it's the MQTT test which is failing. |
Couple of problems I think:
|
I will try releasing a new version of Mosca this evening or tomorrow. I am closing moscajs/mosca#36. |
Fixed making TrieAscoltatore default - it was missed in ascoltatori.build Although it fails on Travis, the tests now pass for me on my local machine with mosca (persistence branch) using the version of Ascoltatori on this pull request. That's because Mosca on Travis won't have these changes. |
I have published the persistence branch, it is released as 0.7.2. |
Seems like mosca is pulling in ascoltatori 0.7.4 instead of the version on this branch: mosca@0.7.0 node_modules/mosca |
This is strange as on my machine I get this: mosca@0.7.2 node_modules/mosca (and the build succeeds) |
|
It is how NPM work for Mosca to fetch a new version of it from scratch. However it is not normal that you have such different behavior on travis and on your machine. |
I guess that build was run when Mosca was at 0.7.0 then? |
Picks up 0.7.2 now so that's sorted. |
Hmm. This seems relevant? https://github.com/isaacs/npm/issues/2063 |
Ah I wonder if it's because I still have version at 0.7.2 in the package.json in this branch. Doesn't really explain why it does it on my machine. |
Obviously not :( Very strange |
Ah got it Cycles are handled using the property of node's module system that it walks up the directories looking for node_modules folders. So, at every stage, if a package is already installed in an ancestor node_modules folder, then it is not installed at the current location. I happen to have this branch checked out in a node_modules directory! That explains the difference with travis. |
…different ascoltatori
OK build is now passing after I made the change in 3e1ac2f:
Is that going to be okay for all pull requests going forward? It basically says tests must pass with mosca using this version of ascoltatori. |
It should be fine, this cyclic dependency between them is very tricky. I just hope your fix solves it for good! |
So I added a script to run the mosca tests too. I think you have a problem with make bail in mosca:
The pipe means errors in the actual test won't be reported. I'll fix the error here by doing an npm install first but just letting you know that the mosca target for 'make bail' probably should change. |
No, we should not run the Mosca tests here. |
OK I'll revert those commit then. |
Btw, your mosca tests on travis (which appear to have succeeded) actually failed: ✖ 1 of 214 tests failed:
(https://travis-ci.org/mcollina/mosca/jobs/8471162) That 'make bail' pipe is causing failures to be missed. |
(if you want to carry on using the pipe, you could use $PIPESTATUS to make sure the errors from mocha are propagated) |
I just saw that. Gosh. It is caused by bunyan, I'll remove it ASAP. If Mosca's tests are failing but Ascoltatori's tests are fine we have a huge problem. |
I have just released Mosca 0.7.3 that solves all these issues. |
OK I reverted the changes which ran the mosca tests too. |
Use '+' as single level wildcard to make it consistent with MQTT.
No description provided.