-
Notifications
You must be signed in to change notification settings - Fork 110
Conversation
@jrschumacher thank you for the pull request. Please move the test file to |
"test": "grunt test", | ||
"build": "bower install; grunt build" | ||
"build": "bower install; grunt build", | ||
"test": "./node_modules/loopback-testing/node_modules/.bin/mocha common/models/test" |
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.
npm test
should run both server and client tests. Please keep grunt test
and rework the Gruntfile: rename the task test
to test:client
, add a test:server
task to run Mocha tests in server/test
, and add a test
task to run test:server
and test:client
.
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.
@bajtos is there a specific way to get grunt to run a command line or should I look at adding grunt-mocha or grunt-exec?
Sure will do. |
@bajtos fixed the grunt issue
How should I be formatted? I left it in the common models because I found your stubs there https://github.com/strongloop/loopback-example-full-stack/tree/master/common/models/test |
Well, those stubs would be for pure unit-tests not using REST transport.
Just move the file, I think the current content is ok for the first version. We can always improve it later. |
quiet: false, // Optionally suppress output to standard out (defaults to false) | ||
clearRequireCache: false // Optionally clear the require cache before running tests (defaults to false) | ||
}, | ||
src: ['common/models/test/**/*.js'] |
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.
This should be server/test/*.js
, the usual convention is to pick up only top-level JS files.
Tests in common
folder would be run by grunt test:common
, but don't worry about that now.
@bajtos I just noticed that I am omitting the user tests since it looks the same |
@bajtos I am done. Not going to do the filter test though. Not enough time. I can rebase my work as needed. |
// Get todo stats | ||
lt.describe.whenCalledRemotely('POST', '/api/Todos/stats', function() { | ||
it('should respond with total, remaining and 1 completed', function() { | ||
lt.it.shouldBeAllowed(); |
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.
You cannot nest lt.it
inside it
blocks. Move it outside.
That's fine. It's better to have some tests than no tests at all.
Cool. I reviewed the code, see the comment above. @ritch Is the test code in this patch using your test helpers correctly? I find nested |
@bajtos I'd be happy to unnest. When I was creating it I assumed the database was cleared after every request as well as the async nature of |
@bajtos here is the proof to the issue with |
I believe the test should call testApp.use(loopback.rest());
helpers.beforeEach.withApp(testApp);
helpers.beforeEach.givenModel(testModel); Note that the helper also creates a single new model instances and stores it on the test context object. I guess this can be improved by splitting the method
Alternatively, you can simply ignore these extra helpers and manually write @ritch thoughts? |
@jrschumacher This is waiting until strongloop-archive/loopback-testing#40 is landed & released, is that correct? |
@bajtos I would suggest that. When it does I'll clear the database before the count so the count makes more sense. |
strongloop-archive/loopback-testing#40 has been landed and released as @jrschumacher could you please finish this pull request into the state where it can be landed? |
db1fe68
to
6970ec2
Compare
@bajtos I believe its good to go please test |
@bajtos ping |
Landed, thank you for the contribution. |
Added some basic tests for example. In progress
Todo:
Todo model
stats
testfilter testsUser model
CRUD tests- the model file looks the same as todo