Wiredcraft / test-backend

39 stars 76 forks source link

Leo v1 #96

Closed xy2009 closed 2 years ago

xy2009 commented 2 years ago

hello!

I have implemented basic functions such as user creation and verification, log records, test conditions and documents, please check the Readme. I am interested in focusing on attention and nearby functions. At present, I submitted PR first. After other functions are completed, I will submit.

Tank you!

xavierchow commented 2 years ago

Thank you for your PR @xy2009 , the team will start to review it soon.

xy2009 commented 2 years ago

hello! I found that there was a bug in the last submission, and I submitted PR again after fixing. Tank you!

xy2009 commented 2 years ago

Thank you for your reply @xavierchow ,I have fixed the authorization bug.

MiffyLiye commented 2 years ago

Hello @xy2009 , I'm going to review the PR, but the code has compilation error when run npm start, can you fix it.

xy2009 commented 2 years ago

Hello @MiffyLiye , Would you please send me the error message, I can fix it, I am currently running normally. it running like this: 1

MiffyLiye commented 2 years ago

@xy2009 I don't have a computer nearby so I cannot provide details, but it's about missing files in ignored folder configured in .gitignore.

xy2009 commented 2 years ago

Hello @MiffyLiye Whether it is the following similar error information: image if it is because the mongodb configuration is wrong, the database configuration in config/default.json can be modified. it's wrong configuration: image

change to right like this: image

You can also refer to the .env.emaple configuration style file of the root directory to add your own local .env loaded the local configuration. My local .env configuration like this: image

If this problem can be fixed through the above two operations.

I'm sorry for the configuration that has not been in detail in readme.md, which brings you a bad experience

xavierchow commented 2 years ago

got the following errors, @xy2009 besides the fix needed, is there a better way to get everybody set up with the same files?

 ~/workspace/recruitment/xy2009/test-backend/mongo-init   leo_v1 ●  yarn start
yarn run v1.22.17
$ npm run compile && node lib/

> koa-rest-admin@1.0.0 compile
> shx rm -rf lib/ && tsc

src/services/LoginLog.service.ts:3:25 - error TS2307: Cannot find module '../lib/BaseService' or its corresponding type declarations.

3 import BaseService from '../lib/BaseService';
                          ~~~~~~~~~~~~~~~~~~~~

src/services/LoginLog.service.ts:101:41 - error TS2554: Expected 0 arguments, but got 3.

101   const serverUser = new ServerLoginLog(LoginLog, 'ServerLoginLog', getServiceMainUrl('serverLoginLog'));
                                            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

src/services/ServerUser.service.ts:3:25 - error TS2307: Cannot find module '../lib/BaseService' or its corresponding type declarations.

3 import BaseService from '../lib/BaseService';
                          ~~~~~~~~~~~~~~~~~~~~

src/services/ServerUser.service.ts:4:32 - error TS2307: Cannot find module '../lib/types' or its corresponding type declarations.

4 import { Id, NullableId } from '../lib/types';
                                 ~~~~~~~~~~~~~~

src/services/ServerUser.service.ts:560:37 - error TS2554: Expected 0 arguments, but got 3.

560   const serverUser = new ServerUser(User, 'ServerUser', getServiceMainUrl('serverUser'));
                                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Found 5 errors in 2 files.

Errors  Files
     2  src/services/LoginLog.service.ts:3
     3  src/services/ServerUser.service.ts:3
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
xy2009 commented 2 years ago

@xavierchow I'm very sorry, the git ignore the /src/lib file,I have repaired, please check, thank you.

xy2009 commented 2 years ago

@xavierchow Hello! I have submitted the relevant functions of the expected implementation and submitted PR again, thank you.

MiffyLiye commented 2 years ago
  1. The error response HTTP status code is always 200 OK, while the code in response body uses 401, 404, etc. that is not typical RESTful style, can you explain why you choose this API style?
  2. A lot of unit tests fail, I think it's another case that something only work in your local environment, can you list some common practice that can find this kind of failure early?
xy2009 commented 2 years ago

@MiffyLiye Hello! Sorry for the operation of the project to increase your trouble for you.

For the first question:

  1. When the project was initially constructed, when I encountered illegal data or operation, the ERROR was thrown out. At this time, the HTTP status code was consistent with the return of the error, but the Message information was lost during the transmission process; The follow-up Jest also encountered some problems. CTX added Error Result to deal with it. At this time, the information problem was solved, which caused the state code to be inconsistent.I didn't pay much attention to this issue at that time.
  2. I create some customized status codes are not supported by the HTTP status code. If I want to transfer the information passed to the call terminal, it will cause the state code to be inconsistent.
  3. KOA is used less. I didn’t know that there was a throw method on the CTX before. It can be used it and change the customError structure to solve the above problems. I will submit it again to repair this problem.

For the second question: It is known that the report of the test case reports the error information due to some data needs to be initialized in advance, Three ways to think of:

  1. Optimized test case structure, the original test function is processed by descibe function according to the logic of the test case to ensure the correctness of the logic.
  2. Use a fixed MOCK data for testing. After the test is completed, the relevant data is deleted to avoid conflicts in subsequent testing.
  3. Using Git Commit HOOK for this modified file -related test case to ensure that it has not had an abnormal effect on existing functions.
xy2009 commented 2 years ago

@MiffyLiye I have fixed the above two bugs, please check. Thank you.

xavierchow commented 2 years ago

Thanks all, the PR is good enough for us to move forward.

@xy2009 I'm closing this PR, we will get back to you soon.

xy2009 commented 2 years ago

@xavierchow @MiffyLiye @bsdelf Thanks all, I'm glad to communicate with everyone.