Commit defs #758
Commit defs #758
Conversation
|
|
||
| all: lib/defs.js |
There was a problem hiding this comment.
I replaced the default "all" target with an "error" target because we don't want people running make and unintentionally generating new lib/defs.js
| coverage: $(NYC) lib/defs.js | ||
| $(NYC) --reporter=lcov --reporter=text $(_MOCHA) -u tdd -R progress test/ | ||
| coverage: $(NYC) | ||
| $(NYC) --clean --reporter=lcov --reporter=text $(_MOCHA) -u tdd --exit -R progress test/ |
There was a problem hiding this comment.
Instead of the clean target deleting the .coverage directory, I thought it better to have nyc do it.
Mocha was hanging so I added the --exit argument
There was a problem hiding this comment.
What is causing Mocha to hang? It's generally a good idea to avoid --exit, because otherwise we can miss problems with resource disposal.
There was a problem hiding this comment.
I'll see if I can work it out. Most likely a connection to the broker not being closed in one of the tests.
There was a problem hiding this comment.
There are loads of test where the connection isn't being closed. Really this should be done on both success and failure, so it will take a while to fix them all. I'll create a separate issue for it and leave the --exit for now.
| @echo "HTML report at file://$$(pwd)/coverage/lcov-report/index.html" | ||
|
|
||
| lib/defs.js: clean bin/generate-defs test |
There was a problem hiding this comment.
I wanted to be sure the new lib/defs.js were tested post generation
There are some good arguments for committing the generated lib/defs.js file
I've also decided to include the amqp-rabbit-0.9.1.json specification. It's not necessary, but only a small increase to the overall package size and might prove useful for someone trying to understand why an operation is missing