search

freeCodeCamp / pantry-for-good

An open source food bank logistics and inventory management tool
Other
395 stars 189 forks source link

Add unit tests for list API of customer controller #406

Closed tsukimi2 closed 5 years ago

tsukimi2 commented 5 years ago

Tried to submit this pull request of adding unit tests for the list API of customer controller. The tests all pass when run under my local environment. However, one of the tests failed when submitted to this pull request (see below). Would you be able to help take a look at the cause of the problem? Thanks.

1) User Api signup does not allow signup if email address already has an account: Error: expected 400 "Bad Request", got 200 "OK" at Test._assertStatus (node_modules/supertest/lib/test.js:268:12) at Test._assertFunction (node_modules/supertest/lib/test.js:283:11) at Test.assert (node_modules/supertest/lib/test.js:173:18) at Server.localAssert (node_modules/supertest/lib/test.js:131:12) at emitCloseNT (net.js:1568:8) at _combinedTickCallback (internal/process/next_tick.js:77:11) at process._tickCallback (internal/process/next_tick.js:104:9)

jspaine commented 5 years ago

Hi, sorry for the delay, I'll try to have a look at this later. Just triggered a build to see if it's also failing without any changes.

The tests look good now, if you get it ready I'll merge it with the build failure, because I don't think it's caused by your changes.

jspaine commented 5 years ago

Those user integration tests were really flaky, it would fail locally running only them but work when they ran after some other tests. I guess using the express app directly without calling setupPassport might have caused it. It's hard to debug when you can't see any logs, be nice to find a way to fix that.

Can you try pushing something to your branch to see if it works now?

tsukimi2 commented 5 years ago

I see. If the check fail is not due to changes in my code, In that case, let me close the current PR, uncomment all the other commented test cases in the current customer.spec.js file, and then submit a new PR based on that. Should have time to do it either tomorrow night or during the weekend. Thanks for checking the issue.