[ChaosCenter]: Add unit test code to chaoshub package in graphql server

[namkyu1999]

Proposed changes

Add unit test code to chaoshub package. If this approach is valid, I will adopt this approach to all backend components.

This PR changes below...

1. Add unit test code to chaoshub/service.go
2. Change go.mod
   2.1. Add github.com/stretchr/testify library for better mocking
   2.2. Change version of go.mongodb.org/mongo-driver v1.8.2 to v1.11.4 (No side-effect)
3. Move the GetIconHandler function in the chaoshub package to the rest_handlers package

You can check test coverage like below.

cd ./litmus-portal/graphql-server/pkg/chaoshub
go test -coverprofile cover.prof .

I achieved 80% of test coverage now

And you can check UI from this line
go tool cover -html=cover.prof -o cover.html

Issue No. #3892

Types of changes

What types of changes does your code introduce to Litmus? Put an x in the boxes that apply

• New feature (non-breaking change which adds functionality)
• Bugfix (non-breaking change which fixes an issue)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation Update (if none of the other choices applies)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

• I have read the CONTRIBUTING doc
• I have signed the commit for DCO to be passed.
• Lint and unit tests pass locally with my changes
• I have added tests that prove my fix is effective or that my feature works (if appropriate)
• I have added necessary documentation (if appropriate)

Dependency

• Please add the links to the dependent PR need to be merged before this (if any).

Special notes for your reviewer: @amityt @S-ayanide @imrajdas @gdsoumya

[namkyu1999]

It takes approximately 15 seconds for testing now. Because In Chaoshub service component, They called real rest calls like cloning git repository. I'm not mocking that rest calls because, in that component, They have logic that processes files based on download(GET request).

[amityt]

It takes approximately 15 seconds for testing now. Because In Chaoshub service component, They called real rest calls like cloning git repository. I'm not mocking that rest calls because, in that component, They have logic that processes files based on download(GET request).

This is awesome @namkyu1999 . 🚀🚀
Since you are not mocking the go-git handlers which are used to do the git operation like clone, sync etc. Can we add a cleanup script to delete these cloned repositories once the tests are over? cc: @imrajdas

[namkyu1999]

It takes approximately 15 seconds for testing now. Because In Chaoshub service component, They called real rest calls like cloning git repository. I'm not mocking that rest calls because, in that component, They have logic that processes files based on download(GET request).

This is awesome @namkyu1999 . 🚀🚀 Since you are not mocking the go-git handlers which are used to do the git operation like clone, sync etc. Can we add a cleanup script to delete these cloned repositories once the tests are over? cc: @imrajdas

Sure! I'll mention it again after I add it.

[namkyu1999]

I added defer function to clean up cloned repository, @amityt

[gdsoumya]

Looks good, can we also add a ci job in the github actions to run the uint tests? That would make it possible for the reviewers to know the tests are working/passing without having to run them locally.

[namkyu1999]

Looks good, can we also add a ci job in the github actions to run the uint tests? That would make it possible for the reviewers to know the tests are working/passing without having to run them locally.

Sure! And I'm now writing docs about the test approach, I will update this PR after sharing docs.

[namkyu1999]

I wrote 'LitmusChaos Unit Test Strategies'.
Here's a link: https://docs.google.com/document/d/10kEGZr_lxH8HBNC4YYVZ-dYSkEABQskZwLsb1gPAjO8/edit?usp=sharing

Anyone can comments to make better Docs. I'm currently in the process of modifying the tests in the ChaosHub package according to the docs.

[namkyu1999]

I finished writing unit test codes in ChaosHub pkg. I achieved 3.3% code coverage of the entire graphql-server. Each coverage is like the below.

[namkyu1999]

In GitHub Actions CI build, I separated unit-tests from backend-checks. Also, All backend components image build will be executed after unit-tests. If backend-checks are not executed successfully, unit-tests are not executed. Likely, If unit-tests are not executed successfully, the image build of backend components is not executed.

Check this commit : fix: fix GitHub Actions config

[S-ayanide]

Can you add private=true test cases for each of the test types (Sync, clone, pull, etc) too! In the same handler and assert it too.

[namkyu1999]

@S-ayanide
It's cool but, our current unit test runs an actual git clone job so we need a real private repository and private key to run this job. IMO It's not reasonable for testing with a real private key in the unit test. if we want to test private repository test cases, we need to mock the git clone parts (make a new interface to run GitClone) and not run the actual git clone. What do you think?

[gdsoumya]

@namkyu1999 maybe we can do a negative test with random creds and make sure we get an unauthorized response, because at the end of the day the git clone is handled by go-git and we don't have to test that what we need to test is whether we are calling the go-git package with proper data and imo if we can even get an unauthorized response for a valid private git repo we should be good.

Also if we absolutely want to test private repos we can add a github secret to this repo and then use it as an env in ci for the test to read. Though this may have security concerns, we can minimize it to some extent by limiting the access of the test creds to only 1 dummy repo

[namkyu1999]

Okay, then I will make that test cases too!

[amityt]

Yes , @namkyu1999 you can add the negative test cases for the git functionalities for now. And for the positive test cases that @gdsoumya mentioned, we can take it in a different PR. We can break these test case PRs in different chunks for more flexibility in tracking.

[namkyu1999]

@S-ayanide , I added negative test cases and i will add positive test cases in different pr just like @amityt mentioned.

[S-ayanide]

Awesome, +1 to @gdsoumya's points, asserting the error response would also count as valid in this case. That could be the first approach we take.

[S-ayanide]

LGTM

[gdsoumya]

LGTM