Skip to content
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

Test file didn't do the test #897

Closed
Killea opened this issue Aug 19, 2018 · 6 comments
Closed

Test file didn't do the test #897

Killea opened this issue Aug 19, 2018 · 6 comments

Comments

@Killea
Copy link

Killea commented Aug 19, 2018

If I change some test code to

describe('GET /random-url', () => { it('should return null', (done) => { request(app) .get('/reset') .expect(null, done()); }); });
The test can still pass. I don't think this is correct.
If it were correct, what's the meaning of this test script?

@Killea
Copy link
Author

Killea commented Aug 19, 2018

I found the reason, the code should be
describe('GET /random-url', () => { it('should return 403', (done) => { request(server) .get('/reset') .expect(404, done ); }); });
The 'done' function should be without brackets. Other projects fork from this project have the same problem. :(

@YasharF
Copy link
Collaborator

YasharF commented Aug 19, 2018

Brackets? Do you mean parentheses? The parentheses in done() seem to be consistant with the usage in https://github.com/visionmedia/supertest/blob/master/test/supertest.js#L359, is that not the case?

@Killea
Copy link
Author

Killea commented Aug 19, 2018

I think they are different. For the code I posted, if you use done(), actually it's will always pass the test what ever the expect it is. I mean, you cannot get the fail result.
For example, if you expect 200, but the server returns 500, the test still pass because the done() parameter, it should be done.
English is my second language, maybe I didn't explain this clearly.
Just modify a expect function, change 200 to 300, the test will still pass.

@Killea
Copy link
Author

Killea commented Aug 19, 2018

https://github.com/visionmedia/supertest/blob/master/test/supertest.js#L26, in this line , they used done not done(). This is what I want to say, in hackathon-starter code it is done(), which makes the test always pass.

@Killea
Copy link
Author

Killea commented Aug 19, 2018

https://github.com/sahat/hackathon-starter/blob/master/test/app.js#L8 For this line it should be
.expect(200, done);, otherwise even if the server returns 500 ,the test still pass.

@YasharF
Copy link
Collaborator

YasharF commented Aug 19, 2018

Thank you.

Fixed with 623c2a6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants