Segregate the tests and add mock responses and corresponding helper functions #47
Conversation
… state for unit tests
LakshSingla
changed the title
LakshSingla
force-pushed
the
test-suite
branch
from
624a563
to
55e0fdd
Compare
There was a problem hiding this comment.
a few tiny inline comments here, but nothing big.
It might be nice to define the bundles and meta files in the test readme as well.
Ideally this should pass on travis before we merge, and #54 should be fixed (but it sounds like @amand1996 has this in hand!)
Overall the style looks great.
test/mocha/id-constraints.coffee
Outdated
|
|
||
| {service, olderEmployees} = new Fixture() | ||
|
|
||
| describe 'ID constraints', -> | ||
|
|
||
| # MOCK HERE |
There was a problem hiding this comment.
No, I should remove this
| @@ -173,7 +174,8 @@ testIDResolutionAgainst = (service, extraTests = {}) -> | |||
| job.decay.should.be.above 50 | |||
| return undefined | |||
|
|
|||
| describe 'Service', -> | |||
| # BOTH (uses above function, mock accordingly) | |||
There was a problem hiding this comment.
Not sure what this means, sorry - could you clarify?
There was a problem hiding this comment.
So the test used the function defined above (which was a standalone function). Therefore I added this in case if anyone added mock inside the function, that it would affect the corresponding test too.
| it 'should have the expected fields for Employee', (done) -> | ||
| promise.then null, done | ||
| promise.then (sfs) -> | ||
| sfs.Employee.should.eql expected | ||
| done() | ||
| return undefined | ||
|
|
||
| # BOTH |
There was a problem hiding this comment.
Hmm, q: why is this file #BOTH when others have a method bothTests() instead?
There was a problem hiding this comment.
So the tests run as both integration as well as unit tests, I forgot to add the bothTests() annotation. bothTest() returns true always, so it doesn't make a difference. It was added for consistency with other tests and an off chance that we need to change the default behavior when both tests should run. Thanks for pointing this out!!
|
booo, travis sucks right now. so long as it passes test locally we can merge even if it's not great on travis. That travis error is happening everywhere interminey, not just imjs. It's definitely not imjs's fault. |
PR authors: Laksh Singla
Please describe your PR:
This PR aims to improve upon the current state of tests by segregating the tests into Unit Tests and Integration Tests. Moreover, the required responses will be mocked for unit tests. Further extension will allow these mocks to update with the help of a script.
Reviewers: Aman Dwivedi, Yo Yehudi
Review checklist: