Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test backend yingjun #101

Open
wants to merge 33 commits into
base: master
Choose a base branch
from

Conversation

yingjun
Copy link

@yingjun yingjun commented May 30, 2023

Tech used:

  1. NodeJS & NestJS framework & Prisma orm
  2. Mongodb

How to run

  1. start mongodb container use provided docker-compose file docker-compose up -d
  2. run nvm use
  3. and run yarn dev
  4. go to http://localhost:3000/api-docs you can use this swagger page test api

High level design diagram

Class Diagram

yingjun added 30 commits May 23, 2023 15:11
1. nodemon for hot reload during development.
2. use prisma as orm framework
3. use nestjs as base framework
4. additional lib(s): bytenode protect code in production.
…which is user can updated with existed email address.
…equest, they should generated and handle by database orm
@bsdelf
Copy link

bsdelf commented May 31, 2023

In the high-level design diagram, what's the meaning of "->" arrow, is it a directed association?

const { method, originalUrl: url } = req;

res.on("finish", () => {
const startTime = now("micro");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you explain why the startTime variable is placed inside the finish callback

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I meant to 'Aggregation' I think in current requirements user controller should has strong dependency with user service.
  2. this a bug, I found this issue yesterday after I commit. it should be in upper block. and the reason why use micro time b/c of it is too fast when executing on local. if you seconds it will always be 0ms.

),
() => this.dbHealth.isHealthy("mongodb"),
() =>
this.disk.checkStorage("diskStorage", {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why monitor disk storage in the backend service?
does it store anything on disk to support certain feature?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not really, just my coding routine. actually I just meant to monitor the database status.

mongo1:
image: mongo:5
container_name: mongo1
command: ["--replSet", "my-replica-set", "--bind_ip_all", "--port", "30001"]
Copy link
Member

@MiffyLiye MiffyLiye May 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does default configuration support replica set?
where to config the database if we want to use single node mongo db in local, while use replica set in QA / Staging / Production?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here is the story.
this is my first time use Prisma orm, I know it is bit risk use new lib/framework which I don't familiar during interview project. But I always want to learn something new from each interview. I spent few hours read the doc and I found it is only supports replica. Also I think it is generally considered a good practice using MongoDB in a replica set configuration.
and if we really need use single node mongodb, we can easily replaced it with other orm framework, that is why I use interface injected to the userService instead of inject concrete prismaService class. the code can be much more simplified if I directly inject prismaService to userService but I think it lots the flexibility and maintainability.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's ok to use replica set, does the default configuration support replica set?
where to configure it if another environment uses more nodes or other ports?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's ok to use replica set, does the default configuration support replica set?
Yes, prisma ONLY supports replica.

where to configure it if another environment uses more nodes or other ports?

sorry I forgot push example.env file. We can define DATABASE_URL in .env or pass as ENV var.
Screenshot 2023-06-02 at 16 21 02

@MiffyLiye
Copy link
Member

In controller level test, dependency in service level is mocked.
In service level test, an in memory database is used as a mock database.
Some test styles only mock a few dependencies, e.g. external API (e.g. WeChat Service API), and not mock dependencies like database.
Can you compare the benefit and cost of those test styles?
What do you think when should we use mock?

@yingjun
Copy link
Author

yingjun commented May 31, 2023

pros: mock service layer during controller testing we can focusing controller's behavior in isolation. Also some times service layer is not implemented yet in this case we need mock it.
cons: since the service dependency is mocked, it may not catch integration issues between the controller and the service. we need e2e test for whole calling chain.

pros: for the service testing sometimes we need mock database since database server is not accessible, also the memory db is much more faster, if we have lots of tests it may has very long execution time. also real database is highly relay on environment.
cons: since memory db is emulate the behavior of a real database, which can introduce the risk of missing potential issues. in this case we need test against with real database in qa/uat/stg env before go to prod.

expect(createdUser.id).toBeDefined();
userEquals(newUser, createdUser);
});
it("should create user failed, duplicated email address", async () => {
Copy link
Member

@MiffyLiye MiffyLiye Jun 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in which test setup step is index created?
in deployment process of QA/Staging/Production, how to create required index?
and how to know index is created successfully, or failure reason?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is no index added in this project. I think it is not good practices add index at beginning of the new project, unless it has fairly a mount of preloaded data in db otherwise Create index in empty db may cause slowness insertion.

To create required index usually it should be done by db migration scripts instead add indexes directly on database, and we can use post deployment script check if index created properly. Or use db monitor tool, if it is not first release we also need do the regression test make sure everything works fine.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you declared user's email field to be unique, even without index, the app still checks in slower way to find if there is another user with same email when insert / update.
would the index make insertion faster in this case?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you declared user's email field to be unique, even without index, the app still checks in slower way to find if there is another user with same email when insert / update.
would the index make insertion faster in this case?

lol I think I misunderstood your question previously and also in my code I did something wrong, I didnt mean to add unique index, I'm trying to add comment to email field. it should be //unique instead of @unique.
for the database index it really depends on cases. so I didn't try add any in this project.

@MiffyLiye
Copy link
Member

pros: mock service layer during controller testing we can focusing controller's behavior in isolation. Also some times service layer is not implemented yet in this case we need mock it. cons: since the service dependency is mocked, it may not catch integration issues between the controller and the service. we need e2e test for whole calling chain.

there is one e2e test in test/app.e2e-spec.ts, but just a hello world api test.
what blocks you from adding e2e test there?

pros: for the service testing sometimes we need mock database since database server is not accessible, also the memory db is much more faster, if we have lots of tests it may has very long execution time. also real database is highly relay on environment. cons: since memory db is emulate the behavior of a real database, which can introduce the risk of missing potential issues. in this case we need test against with real database in qa/uat/stg env before go to prod.

with docker, it's easy to setup database server. there are 10 ~ 20 tests in this project, not so many.
what else blocks you from using real database?
what do you think about using real database in e2e test in local or CI runner, before deploy to shared environment (qa/uat/stg)?

@yingjun
Copy link
Author

yingjun commented Jun 1, 2023

there is one e2e test in test/app.e2e-spec.ts, but just a hello world api test. what blocks you from adding e2e test there?

lol. it is default e2e test. when I finished the interview test there is no time to do it, and it was 2 days delayed so just no time to do it. I don't want to find any excuse but I had really long week, we release new product last week. I start this project early but don't have time to do it until weekend.

with docker, it's easy to setup database server. there are 10 ~ 20 tests in this project, not so many.
what else blocks you from using real database?

Actually there is nothing blocks me using real database, just provided another option here. Personally I prefer to use memory db b/c of it is fast and easy to maintain, also it if we use real database it may cased some dirty read issue which may fails unit tests and it will be wasting time on debug the failure tests.

what do you think about using real database in e2e test in local or CI runner, before deploy to shared environment (qa/uat/stg)?

Pros: using a real database during E2E testing helps replicate the actual production environment more accurately. This ensures that you are testing against the same data storage and retrieval mechanisms that your application will use in production.
Cons: using real databases during E2E testing provides more accurate testing scenarios, but it requires careful management of test data and consideration of performance factors. It needs careful test data management. e.g. test should not rely on specific data existing in the database, data should create on fly to prevent dirty read fails the tests. Also needs consider test performance if we have lots of tests in project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants