created unit test for the k8 describe package #287 #289

Philip-21 · 2023-04-07T15:23:08Z

Signed-off-by: Philip-21 [email protected]

Description

The PR fixes for #287

I created a k8 unit test using for the describe package .

The role of mockClient is to provide a mock implementation of the meshkitkube.Client interface, which allows the Describe() to be tested in isolation from the Kubernetes API. Instead of interacting with the real Kubernetes API, the function can use the Describe() mock implementation provided by mockClient to simulate interactions with the API and verify that the function behaves correctly under different conditions.
The MockDescriber is used in the implementation of the Describe() to create a mock output that is returned when the
function is called with specific options
it takes in a Client object and a DescriberOptions object, and returns a string and an error

The approach of using a K8 api can slow down the test suite and make it more difficult to maintain.
Using mock objects and dependencies allows us to isolate the specific code we want to test and provide controlled inputs and outputs, without the need to interact with the actual Kubernetes API. This makes the tests faster, more reliable, and easier to maintain.
I added some comments where necessary to help understand the code better .

welcome · 2023-04-07T15:23:11Z

Yay, your first pull request! 👍 A contributor will be by to give feedback soon. In the meantime, please review the Layer5 Community Welcome Guide and sure to join the community Slack.
Be sure to double-check that you have signed your commits. Here are instructions for making signing an implicit activity while peforming a commit.

Philip-21 · 2023-04-07T15:31:55Z

I am also working on the other packages, i am still going through the code base to understand it better ,so i could write the unit test.

Aisuko

There are two suggestions please take a look. And If we can make this happen in this repo. We can add test cases to other repos to increase the coverage.

Aisuko · 2023-04-10T07:19:40Z

utils/kubernetes/describe/describe_test.go

+	}
+
+	//run test cases
+	for _, tc := range testCases {


Thanks for your contribution. We really need a unit test for all the repos. All the code is good and clean but there is consideration about using it to run all the test cases manually.
Please check the code below, we prefer to use it this way.

https://github.com/meshery/meshery-operator/blob/1ace1d307c2278325b3c7b9c741f7dc7af912529/api/v1alpha1/meshsync_types_test.go#L62-L95

ok this method seems new to me tho, but i would see how i can implement it . I would reach out incase i have issues or i am stuck.

Aisuko · 2023-04-10T07:22:58Z

utils/kubernetes/describe/describe_test.go

+)
+
+/*
+  The MockDescriber is used in the implementation of the Describe function to create a mock output that is returned when the


Nice code comment and this is what we need. So, may you help add a code comment to the describe.go file. I believe if you have written a test case for it you have known what exactly it does.

Sure, i can do this

Aisuko · 2023-04-10T07:26:46Z

By the way, please sign off your commit, more detail https://github.com/meshery/meshkit/pull/289/checks?check_run_id=12627332331

Revolyssup · 2023-04-10T08:33:16Z

utils/kubernetes/describe/describe_test.go

+				DescribeFunc field is a function that takes in a Client
+				object and a DescriberOptions object, and returns a string and an error
+			*/
+			DescribeFunc: func(client *meshkitkube.Client, options DescriberOptions) (string, error) {


What is it that we are testing over here? You can't define the function that is to be tested inside of the test. This function is to be tested

meshkit/utils/kubernetes/describe/describe.go

Line 91 in 3f37557

func Describe(client *meshkitkube.Client, options DescriberOptions) (string, error) {

Ideally, you need to create a mock http server using httpmock and configure it to mock kube api server's response.

The DescribeFunc wasnt used for testing it was the MockClient and tc.Options were used as a parameter for testing. Instead DescribeFunc is a testcase field that creates a MockDescriber for testing.
func (m *MockDescriber) Describe(client *meshkitkube.Client, options DescriberOptions) (string, error) { return m.DescribeFunc(client, options) }
which takes in parameters mockClient and tc.Options
simply returns the result of the DescribeFunc instance
The DescribeFunc creates a mock output that is returned when the
function (Describe()) is called with specific options.

Ok thanks for the suggestion, i would create a mock http server response for the k8 api.

I agree with you here but I believe these codes are still useful. If we assume that MockDescriber has a function named Describe mock the real Describereturns it will be useful. Maybe he does not want to mock any of the API server's responses and only wants to test the function itself. Although, it looks like the code does not test the target function features. We can change a little bit of this code to test the edge situation for the real Describe function.

Is that sound good guys?

Maybe we can discuss an outline for the mock test. It determines what's your target. After we define the scope, it would have a clear roadmap for mocking the test suite.

The approach of using a K8 api to return any response can slow down the test suite and make it more difficult to maintain.
like @Revolyssup had said i should create a mock response for the api , which is what i will implement .

I suggest you to use https://github.com/kubernetes/client-go/blob/8005e0d28ba2db8a60bb8dbe8a5a01fed4872f67/rest/fake/fake.go#L46-L48 to mock the Kubernetes api-server response.

And PrependReactor() may useful here. https://stackoverflow.com/questions/69740891/mocking-errors-with-client-go-fake-client

Ok thanks I will look into it .

You are welcome. It is useful for all our repos. If you need any help, please let me know. https://github.com/kubernetes/client-go/blob/8005e0d28ba2db8a60bb8dbe8a5a01fed4872f67/testing/fake.go#L103

Signed-off-by: Philip-21 <[email protected]>

Signed-off-by: Philip Obiora <[email protected]>

Signed-off-by: Philip-21 <[email protected]>

Signed-off-by: Philip Obiora <[email protected]>

stale · 2023-06-08T14:34:22Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

leecalcote · 2023-06-08T14:58:15Z

@Aisuko thanks for helping getting this PR into shape and ready for merge. Do you mind giving this another review?

@Philip-21 is there any outstanding feedback to address?

// @nebula-aac

Aisuko · 2023-06-09T00:14:23Z

@Aisuko thanks for helping getting this PR into shape and ready for merge. Do you mind giving this another review?

@Philip-21 is there any outstanding feedback to address?

// @nebula-aac

You are welcome @leecalcote. I saw that @Philip-21 remove all the files.

Philip-21 · 2023-06-11T20:15:54Z

@Aisuko thanks for helping getting this PR into shape and ready for merge. Do you mind giving this another review?

@Philip-21 is there any outstanding feedback to address?

// @nebula-aac

This Pr had DCO and commit issues so I decided to remove all files
The issue is continued on #319
I'm Still communicating with Ashish on the next step .

stale · 2023-07-26T23:34:11Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale · 2023-08-06T01:27:54Z

This issue is being automatically closed due to inactivity. However, you may choose to reopen this issue.

Philip-21 changed the title ~~created unit test for the k8 describe package~~ created unit test for the k8 describe package #287 Apr 8, 2023

Philip-21 changed the title ~~created unit test for the k8 describe package #287~~ created unit test for the k8 describe package Apr 8, 2023

Philip-21 changed the title ~~created unit test for the k8 describe package~~ created unit test for the k8 describe package #287 Apr 10, 2023

Aisuko self-requested a review April 10, 2023 06:59

Aisuko requested changes Apr 10, 2023

View reviewed changes

Philip-21 force-pushed the k8-describe-package-test branch from 54b58ee to 6e9756d Compare April 10, 2023 08:22

Revolyssup requested changes Apr 10, 2023

View reviewed changes

Philip-21 and others added 2 commits April 12, 2023 07:19

Merge branch 'master' into k8-describe-package-test

e9d6281

added new mockclient to hande http request and responses

af7fda8

Philip-21 force-pushed the k8-describe-package-test branch from 057b135 to af7fda8 Compare April 19, 2023 02:05

Philip-21 and others added 4 commits April 19, 2023 03:21

removing a packae

39ea387

Signed-off-by: Philip-21 <[email protected]>

Merge branch 'master' into k8-describe-package-test

e71a677

remove old file

1c9bbb0

Signed-off-by: Philip Obiora <[email protected]>

adding mockclient to access k8 api and return mock response

fd006e4

Signed-off-by: Philip-21 <[email protected]>

Philip-21 force-pushed the k8-describe-package-test branch from 52dff0d to fd006e4 Compare April 19, 2023 09:06

Philip-21 added 3 commits April 19, 2023 02:09

delete

e6da55a

Signed-off-by: Philip Obiora <[email protected]>

Adding MockClient to handle requests Return Response

3fc26ce

Signed-off-by: Philip Obiora <[email protected]>

delete

c985781

Signed-off-by: Philip Obiora <[email protected]>

stale bot added the issue/stale Issue has not had any activity for an extended period of time label Jun 8, 2023

stale bot removed the issue/stale Issue has not had any activity for an extended period of time label Jun 8, 2023

stale bot added the issue/stale Issue has not had any activity for an extended period of time label Jul 26, 2023

stale bot closed this Aug 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

created unit test for the k8 describe package #287 #289

created unit test for the k8 describe package #287 #289

Philip-21 commented Apr 7, 2023 •

edited

Loading

welcome bot commented Apr 7, 2023

Philip-21 commented Apr 7, 2023

Aisuko left a comment

Aisuko Apr 10, 2023 •

edited

Loading

Philip-21 Apr 12, 2023

Aisuko Apr 10, 2023

Philip-21 Apr 10, 2023

Aisuko commented Apr 10, 2023

Revolyssup Apr 10, 2023

Philip-21 Apr 10, 2023 •

edited

Loading

Philip-21 Apr 10, 2023

Aisuko Apr 10, 2023

Aisuko Apr 10, 2023

Philip-21 Apr 10, 2023

Aisuko Apr 13, 2023

Aisuko Apr 13, 2023

Philip-21 Apr 13, 2023

Aisuko Apr 13, 2023 •

edited

Loading

stale bot commented Jun 8, 2023

leecalcote commented Jun 8, 2023

Aisuko commented Jun 9, 2023

Philip-21 commented Jun 11, 2023

stale bot commented Jul 26, 2023

stale bot commented Aug 6, 2023

created unit test for the k8 describe package #287 #289

created unit test for the k8 describe package #287 #289

Conversation

Philip-21 commented Apr 7, 2023 • edited Loading

Description

welcome bot commented Apr 7, 2023

Philip-21 commented Apr 7, 2023

Aisuko left a comment

Choose a reason for hiding this comment

Aisuko Apr 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aisuko commented Apr 10, 2023

Choose a reason for hiding this comment

Philip-21 Apr 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aisuko Apr 13, 2023 • edited Loading

Choose a reason for hiding this comment

stale bot commented Jun 8, 2023

leecalcote commented Jun 8, 2023

Aisuko commented Jun 9, 2023

Philip-21 commented Jun 11, 2023

stale bot commented Jul 26, 2023

stale bot commented Aug 6, 2023

Philip-21 commented Apr 7, 2023 •

edited

Loading

Aisuko Apr 10, 2023 •

edited

Loading

Philip-21 Apr 10, 2023 •

edited

Loading

Aisuko Apr 13, 2023 •

edited

Loading