binder
binder copied to clipboard
Update zookeeper to v3.4.12; migrate tests to TAP
Required submodule changes:
- [x] libcbuf
- [x] zookeeper-common
- [x] mname-balance
Jira issue: MANTA-3543
This change updates the version of Zookeeper to 3.4.12 in addition to updating the existing tests to run under TAP instead of nodeunit.
The tests are passing on a dev zone running zk 3.4.12 locally:
🌈 SUMMARY RESULTS 🌈
Suites: 3 passed, 3 of 3 completed
Asserts: 93 passed, of 93
Time: 15s
--------------|----------|----------|----------|----------|-------------------|
File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s |
--------------|----------|----------|----------|----------|-------------------|
All files | 59.67 | 54.72 | 51.85 | 59.61 | |
index.js | 100 | 100 | 100 | 100 | |
recursion.js | 9.76 | 0 | 0 | 9.76 |... 82,383,384,386 |
server.js | 72.64 | 70.32 | 80 | 72.56 |... 18,619,622,625 |
zk.js | 89.6 | 68.18 | 86.96 | 89.6 |... 03,212,213,221 |
--------------|----------|----------|----------|----------|-------------------|
Also... make check
fails. I believe that is because this is still using javascriptlint which can't handle the new "fat arrow" function syntax you are using. Options:
- don't use the new fat arrow syntax for now
- switch to eslint (and optionally prettier for code formatting as well),. rather than javascriptlint and jsstyle (BONUS POINTS)
If you consider the latter, you migth want that as a separate change to have the commits be cleaner. Also let me know and I can show exampels of other repos having made that change.
@bowrocker Is there a jira ticket for this work?
Am I right that this change is just about a patch-level update of ZK? This isn't about the major/minor ZK version upgrades that (a) we eventually want to do and (b) will require us to test and consider upgrade issues.
That's right, the idea is just to get us upgrade for now, then move to (a) and (b); this is from the ZK wiki:
Supported upgrade path: Existing stable 3.4.x versions should be supported, but in order to get the best results, it's strongly advised to upgrade to the latest stable 3.4 version beforehand.
This is a bit of a drive-by review at the moment, but now that we're using modern node and npm, we should check in the package.lock file as generated from the sdcnode release that we're building with.
Good call, done.
Also...
make check
fails. I believe that is because this is still using javascriptlint which can't handle the new "fat arrow" function syntax you are using. Options:1. don't use the new fat arrow syntax for now 2. switch to eslint (and optionally prettier for code formatting as well),. rather than javascriptlint and jsstyle (BONUS POINTS)
If you consider the latter, you migth want that as a separate change to have the commits be cleaner. Also let me know and I can show exampels of other repos having made that change.
I went with the first option for now, but I will follow up with the latter once this PR is merged.