-
Notifications
You must be signed in to change notification settings - Fork 237
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
Basic sanity check test suite #299
Basic sanity check test suite #299
Conversation
so this isnt working because in the test, the app doesnt stop - it just keeps listening. I'm working on splitting the server apart from the running of the server, so the test can control the start and stop of the server |
gulp.task('mocha', function () { | ||
return gulp.src(['test/**/*.js'], { read: false }) | ||
.pipe(mocha({ reporter: 'spec' })) | ||
.on('error', gutil.log) |
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.
I think you can use:
.pipe(mocha({reporter: 'spec'}))
.once('error', () => {
process.exit(1)
})
.once('end', () => {
process.exit()
})
https://github.com/sindresorhus/gulp-mocha#test-suite-not-exiting
574e720
to
187449d
Compare
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.
Just a couple of small things. App doesn't need to listen in the test script, and Travis is running the suite twice
|
||
before(function () { | ||
app = require('../../server') | ||
server = app.listen(3000) |
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.
I don't think the server needs to be listening at this point. supertest
can just accept the app
variable and handles the listening and un-listening itself. So you can delete var server
, server = app.listen(3000)
and the entire after()
hook and it should still run
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.
how can supertest make the server close?
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.
In my initial research (see the thread above on github) it wasn't closing, and that was causing problems
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.
@joelanman Was that not before you stopped the server.js from doing the listening and put it in run-server? If you try removing that listen, you should see it still works and also stops ok. It seemed to when I tried it a moment ago. To quote supertest:
You may pass an http.Server, or a Function to request() - if the server is not already listening for connections then it is bound to an ephemeral port for you so there is no need to keep track of ports.
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.
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.
These appear to be the relevant lines in supertest which listen and close respectively:
https://github.com/visionmedia/supertest/blob/master/lib/test.js#L59
https://github.com/visionmedia/supertest/blob/master/lib/test.js#L126
@@ -9,7 +9,7 @@ | |||
"scripts": { | |||
"start": "node start.js", | |||
"lint": "standard", | |||
"test": "npm run lint" | |||
"test": "gulp test && npm run lint" |
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.
Travis is running the test suite twice, because Travis.yml is doing:
script:
- gulp test
- npm test
But npm test
already runs gulp test
. I would suggest updating Travis.yml to only run npm test
request(app) | ||
.get('/') | ||
.expect('Content-Type', /text\/html/) | ||
.expect(200, done) |
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.
Now that you're calling done()
in the end method, it needs to be removed from the expect
above...
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.
Thanks @andymantell, that gets rid of the double callback!
message from supertest, I hadn't spotted that.
b7903ee
to
2c4eafe
Compare
…ks run across nodejs 4, 5 & 6
App doesn't need to listen in the test script as supertest accepts the app variable and handles the listening and un-listening itself. This also removes the needs for the after block to stop the server.
4e4523b
to
49965cc
Compare
Looks good to me. Thanks @andymantell! |
based on @andymantell's PR (#292)
this moves the tests from Grunt to Gulp