Add tests

[jrschumacher]

Added some basic tests for example. In progress

Todo:

Todo model

• [x] PUT test - need to test updating a todo
• [x] remote method stats test
• [ ] filter tests

User model

• [ ] CRUD tests - the model file looks the same as todo

[bajtos]

@jrschumacher thank you for the pull request. Please move the test file to server/test.

[jrschumacher]

Sure will do.

[jrschumacher]

@bajtos fixed the grunt issue

thank you for the pull request. Please move the test file to server/test.

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

[bajtos]

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.

How should I be formatted?

Just move the file, I think the current content is ok for the first version. We can always improve it later.

[jrschumacher]

@bajtos I just noticed that common/model/user.json is essentially the same as common/model/todo.json. Is this a mistake?

I am omitting the user tests since it looks the same

[jrschumacher]

@bajtos I am done. Not going to do the filter test though. Not enough time.

I can rebase my work as needed.

[bajtos]

Not going to do the filter test though. Not enough time.

That's fine. It's better to have some tests than no tests at all.

I am done.

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 describe.whenCalledRemotely rather unusual, also the spec output of the test suite does not really resemble a specification. Should this use plain supertest instead?

[jrschumacher]

@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 whenCalledRemotely which is why I chose to nest. If it makes sense not to nest I'd be happy to, but I will need some help in determining how to avoid the potential race condition

[jrschumacher]

@bajtos here is the proof to the issue with beforeEach loopback-testing/97c64458

[bajtos]

I believe the test should call beforeEach.givenModel at the beginning. It will install an after handler to remove all existing model instances after each tests.

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 givenModel into two helpers:

• usingModel that checks that the model is configured in the app and installs the afterEach hook to delete all instances
• givenModel that creates the model instance and calls usingModel under the hood

Alternatively, you can simply ignore these extra helpers and manually write afterEach hook that deletes all model instances.

@ritch thoughts?

[bajtos]

@jrschumacher This is waiting until https://github.com/strongloop/loopback-testing/pull/40 is landed & released, is that correct?

[jrschumacher]

@bajtos I would suggest that. When it does I'll clear the database before the count so the count makes more sense.

[bajtos]

strongloop/loopback-testing#40 has been landed and released as loopback-testing@1.1.0.

@jrschumacher could you please finish this pull request into the state where it can be landed?

[jrschumacher]

@bajtos I believe its good to go please test

[ritch]

@bajtos ping

[bajtos]

Landed, thank you for the contribution.