feat: adding artifacts for shopping cart#4
Conversation
virkt25
left a comment
There was a problem hiding this comment.
Tests + A script to setup docker?
f09a45b to
f1fe284
Compare
| }, | ||
| }, | ||
| }); | ||
| await container.start(); |
There was a problem hiding this comment.
How long does it take to start a Redis container?
I am concerned about the overhead this may be adding to npm t. When developing locally, we don't want to (re)start the Redis instance every time we want to re-run our test suite.
Instead, we should start any database instances beforehand. When running on CI, the CI build script should start the instances before running the tests - Travis CI provides hooks for that.
See also #5. Since Travis CI already supports Redis, I think we should use the same approach from #5 and use Travis-CI provided instance of Redist instead of using Docker.
Related best practice: Clean the database before each test.
| userId: 'u01', | ||
| items: [ | ||
| { | ||
| productId: 'p1', |
There was a problem hiding this comment.
Please follow our best practices, in particular Use data builders.
Signed-off-by: Raymond Feng <enjoyjava@gmail.com>
7f29c8f to
bf2af9e
Compare
e8591c5 to
18ae2b6
Compare
virkt25
left a comment
There was a problem hiding this comment.
Will a controller be added in a followup task? Should we use a @hasMany relation between a ShoppingCart and ShoppingCartItem?
| ShoppingCart | ||
| > { | ||
| constructor() { | ||
| super(ShoppingCart, new RedisDataSource()); |
There was a problem hiding this comment.
The DataSource should be injected.
Signed-off-by: Raymond Feng <enjoyjava@gmail.com>
Signed-off-by: Raymond Feng <enjoyjava@gmail.com>
Let's try to add the basic support 1st. |
bin/start-dbs.sh
Outdated
| #!/bin/bash | ||
| if [ -z "$CI" ]; then | ||
| MONGO_CONTAINER_NAME="mongodb_c" | ||
| docker rm -f $MONGO_CONTAINER_NAME |
There was a problem hiding this comment.
is it a dup to remove it again in the start file? I notice that the same cmd is called in stop-dbs.sh.
There was a problem hiding this comment.
Not really, I added stop-dbs.sh so that I can shut down these containers manually for local tests.
There was a problem hiding this comment.
It's just to be sure that such containers are removed. We can call ./stop-dbs.sh instead.
There was a problem hiding this comment.
Not really, I added stop-dbs.sh so that I can shut down these containers manually for local tests.
Oh I see 👌
| @@ -0,0 +1,7 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
should travis call this file after test?
There was a problem hiding this comment.
Travis recycles the containers. We don't have to worry about the stop. BTW, our Travis Mac build uses brew.
There was a problem hiding this comment.
No, Travis pretty much kills the instance that runs the tests once the tests are done ... and we aren't starting a docker containre on Travis but a service.
| /** | ||
| * Item in a shopping cart | ||
| */ | ||
| export class ShoppingCartItem { |
There was a problem hiding this comment.
I am afraid we don't generate OpenAPI schema for a class without decorating it with @model and @property.
virkt25
left a comment
There was a problem hiding this comment.
Minor nitpick. Looks ready to land otherwise.
package.json
Outdated
| "commit": "git-cz", | ||
| "commitmsg": "commitlint -E GIT_PARAMS", | ||
| "docker": "./bin/setup.sh", | ||
| "docker": "./bin/start-dbs.sh", |
There was a problem hiding this comment.
Can we add a docker:stop which calls stop-dbs.sh.
| @@ -0,0 +1,7 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
No, Travis pretty much kills the instance that runs the tests once the tests are done ... and we aren't starting a docker containre on Travis but a service.
Signed-off-by: Raymond Feng <enjoyjava@gmail.com>
Add model/datasource/repository for redis backed shopping carts.
See loopbackio/loopback-next#1481