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

[enhancement]: Test suite with code coverage #43

Open
jwala-anirudh opened this issue Aug 20, 2023 · 27 comments
Open

[enhancement]: Test suite with code coverage #43

jwala-anirudh opened this issue Aug 20, 2023 · 27 comments
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@jwala-anirudh
Copy link
Collaborator

jwala-anirudh commented Aug 20, 2023

Background

FreeApi.app as of today doesn't come with any test cases writtern for the application. I want to keep this as a open issue until we reach the goal of 50% coverage in phase 1.

Phase 2 would be to reach min code coverage of 80% for every subsequent release.

Plan

  • To utilise Jest for unit testing as it provides code coverage as well
  • To utilise Playwright for end-to-end testing
    • Need to use a third-party tool for reporting
    • No option of code coverage available that is built-in
  • Setup necessary packages & configuration to generate coverage report
  • To segregate various modules of FreeApi.app into buckets such as
    • Simple
    • Moderate
    • Complex
  • Test cases writing format must be at levels to ensure the quality of code
    • Unit
    • Integration
    • End-to-End

Outcome

  1. Improved code quality
  2. Avoid unnecessary bugs
  3. Confidence to refactor the code
  4. Easy review of PRs

Additional information

We will not be closing this issue until the stated Phases are achieved. Until then all are welcomed to contribute

@jwala-anirudh jwala-anirudh added enhancement New feature or request help wanted Extra attention is needed labels Aug 20, 2023
@Tusharjagi
Copy link

I didn't see Jest in package.json should I install it?
And for the Integration test can I use a superset npm package

@jwala-anirudh
Copy link
Collaborator Author

jwala-anirudh commented Sep 3, 2023

Hi @Tusharjagi,

Thank you for your interest towards contributing to FreeApi.app currently the test suite setup is under progress. We are commiting the code under branch feat/coverage branch.

You can add jest, supertest & update the test script in package.json

I am also expecting to have a babel configuration json file that will include the presets & plugins necessary for the project. You can start with the base healthcheck API endpoint with a base test case for it to expect 200 as status code on making async call.

I would see this as a good starting point for our enhancement

@Tusharjagi
Copy link

Sure @jwala-anirudh I will investigate it, Firstly I need to understand the code base, and then I will raise a PR for it, If I have another query I will reach out to you ...

@theunhackable
Copy link

how about playwright?

@wajeshubham
Copy link
Collaborator

Any update on this one? playwright seems a good option.

@jwala-anirudh
Copy link
Collaborator Author

Alright, let me get started with playwright then 👍🏻
Thanks Shubham & Ranga

@shrey-dadhaniya
Copy link
Contributor

shrey-dadhaniya commented Jan 18, 2024

Hello @jwala-anirudh ,

I have one question why we are using playwright for api testing as per my understanding it's used E2E testing for web apps

can you explain bit more how we are going to add unit test using playwright or i'm missing something ??

@wajeshubham
Copy link
Collaborator

@shrey-dadhaniya Yes the Playwright is more favourable for browser based E2E tests but it also provides an API testing module which I think is enough for our use case and has really simple apis to just do high level testing.

We could've gone for something like Jest or Mocha but our main focus is on business logic and working on the core functionality rather than robust testing.

However, we understand that doing testing is best practice and it eliminates manual testing work upon doing some major changes.

Playwright API testing reference: https://playwright.dev/docs/api-testing

@jwala-anirudh correct me if I'm wrong

Thanks!

@shrey-dadhaniya
Copy link
Contributor

@wajeshubham Thanks.
@jwala-anirudh can you update here once you finish with setup so i can start with basic and easy tests.
meanwhile i will go through playwright documents for api testing.

@jwala-anirudh
Copy link
Collaborator Author

@shrey-dadhaniya available on feat/coverage branch, we look forward for your contributions
Thanks!

@shrey-dadhaniya
Copy link
Contributor

shrey-dadhaniya commented Jan 25, 2024

@jwala-anirudh@wajeshubham

I have started adding test cases I want to make sure i'm going in right direction.
I'm using mongodb-memory-server to connect mongodb and playwrite to test apis
please check commit. commit is under development but i want your opinion on this. feel free to give you opinion.

Thanks!

@theunhackable
Copy link

theunhackable commented Jan 26, 2024

@shrey-dadhaniya why do you have yarn-lock and package-lock in the commit? should'nt you be using only one package manager

@shrey-dadhaniya
Copy link
Contributor

@shrey-dadhaniya why do you have yarn-lock and package-lock in the commit? should'nt you be using only one package manager

yes, you are right.
it's just draft commit i will remove yarn-lock file in final commit.
The goal for that commit that i'm using playwright correctly or not. the way we decided erilier. once we lock that I will start actual development and then i will remove unnecessary code and files

Thanks for reviewing my code @theunhackable, I really appreciate that 😇 

@shrey-dadhaniya
Copy link
Contributor

Hello @jwala-anirudh , @wajeshubham,

I think we should devide this issue in to multiple smaller issues or PR because it is time consuming task and we can not wait untill everything in done.

My suggestion is we can devide like

  1. front-end
  • core setup (adding packages and example test) 
  • create issue for all apps we have in FE.
  1. back-end
  • core setup (adding packages and example test)
  • create issuesfor all apps we have in BE.

once core setup is done for BE and FE then multiple developers can work separately on different apps

what are your suggestions? 

Thanks.

@wajeshubham
Copy link
Collaborator

@shrey-dadhaniya This issue is only targeting backend testing and that too on a higher level. We are not focusing on the front-end testing as it will be done by the contributors creating the front-end apps which is optional.

@shrey-dadhaniya
Copy link
Contributor

shrey-dadhaniya commented Feb 1, 2024

@wajeshubham Okay.we can still divide BE test cases into multiple PRs. what you think?

  1. first core setup
  2. todo app test
  3. social media app test
  4. e-commerce app test
  5. chat app test
    ...

@jwala-anirudh
Copy link
Collaborator Author

@shrey-dadhaniya That is right, we expect this to be delivered in multiple PRs rather than one.

@shrey-dadhaniya
Copy link
Contributor

shrey-dadhaniya commented Feb 1, 2024

@jwala-anirudh how we want to proceed? I will rais PR for master branch ? and what about git workflow I suggest we should add workflow that run test when anyone create PR. what is your input ? 

@wajeshubham
Copy link
Collaborator

@shrey-dadhaniya Let's first start with the testing. Workflows and pipelines we can implement later. Also, please read the contribution guide, we have mentioned everything regarding how to contribute there.

Thanks

@shrey-dadhaniya
Copy link
Contributor

@wajeshubham yes right but my question is we have already branch feat/coverage that is created by someone my code is dependent on that branch

i can create one branch where i setup playwright and add test case for health function then we merge that branch to master and then we can proceed with app wise PR

what you suggest?

@wajeshubham
Copy link
Collaborator

Ok, raise PR to the feat/coverage branch.

@shrey-dadhaniya
Copy link
Contributor

Todo and health check is done now I think i should take more easy app to test so i'm thinking to add Database Seeding
feature

@wajeshubham@jwala-anirudh give me green flag and i will start working on that

@wajeshubham
Copy link
Collaborator

Yes you can proceed! cc: @jwala-anirudh

@wajeshubham
Copy link
Collaborator

Hi @jwala-anirudh, PR #81 has been merged. Just a reminder to ensure that this branch is prepared for merging into the main branch. Additionally, we'll need to update the README.md to include testing-related instructions. Please let me know if you need any assistance from my side.

Repository owner deleted a comment from shrey-dadhaniya Feb 14, 2024
@shrey-dadhaniya
Copy link
Contributor

what is next we decide to merge master branch and then will continue to add test for other apps and also update readme.

how we are going to proceed from here?

@jwala-anirudh @wajeshubham please advise

@shrey-dadhaniya
Copy link
Contributor

Hello,
anyone working on this?
I'm planning to start with authentication test cases.

@wajeshubham give me green flag to start work

@jwala-anirudh
Copy link
Collaborator Author

Sounds good @shrey-dadhaniya - you can start, please open a draft PR once you complete 50% of work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
Status: In Progress
Development

When branches are created from issues, their pull requests are automatically linked.

5 participants