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

Adding Unit test cases for wrappedNodeFetch and adding github actions #59

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

Hermione2408
Copy link
Member

@Hermione2408 Hermione2408 commented Mar 8, 2023

For code coverage added unit test for wrappedNodeFetch function and integrated git hub actions.

test #354
Closes: keploy/keploy#354

@developer-diganta
Copy link

Hi @re-Tick! Can you please review the PR?

@re-Tick
Copy link
Contributor

re-Tick commented Mar 8, 2023

@Hermione2408 please resolve DCO and commitzen

@Hermione2408 Hermione2408 force-pushed the test#354 branch 3 times, most recently from 5cebecf to 7270469 Compare March 8, 2023 20:16
@Hermione2408
Copy link
Member Author

Can you have a look now @re-Tick

@re-Tick re-Tick added Don't Merge Not to be merged until gsoc results Accepted PR is reviewed and Accepted and removed Accepted PR is reviewed and Accepted labels Mar 9, 2023
@Hermione2408
Copy link
Member Author

Hey @re-Tick , Can you please review the changes now

@re-Tick
Copy link
Contributor

re-Tick commented Mar 14, 2023

a test is failing. I will try these changes locally and request changes if any.

@Hermione2408
Copy link
Member Author

Sure I'll try to resolve too where they are failing


describe('wrappedNodeFetch', () => {
it('should call fetch function with correct arguments in record mode', async () => {
const mockFetch = jest.fn().mockResolvedValueOnce(new Response());
Copy link
Contributor

Choose a reason for hiding this comment

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

Since, outputs are recorded when end event is triggered for response of fetch. We dont need to mock fetch here. We can use the actual fetch of node-fetch

deps: [],
};
createExecutionContext(ctx)
const wrappedFetch = (wrappedNodeFetch(mockFetch) as any).bind({ fetch: mockFetch });
Copy link
Contributor

Choose a reason for hiding this comment

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

And pass actual fetch here. To capture the outputs.

};
createExecutionContext(ctx)
const wrappedFetch = (wrappedNodeFetch(mockFetch) as any).bind({ fetch: mockFetch });
const url = 'http://example.com';
Copy link
Contributor

Choose a reason for hiding this comment

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

We can use keploy's health API "https://api.keploy.io/healthz" here as url

const deps=updatedctx.deps.length;
const responseBody = await response.text();
const recordedOutput = updatedctx.mocks[0].Spec.Res.Body;
expect(mockFetch).toHaveBeenCalledWith(url, options);
Copy link
Contributor

Choose a reason for hiding this comment

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

We also dont need to do this since we use actual fetch of node-fetch

@Hermione2408
Copy link
Member Author

@re-Tick I have made the changes and it is passing all the test cases now.

Hermione2408 and others added 8 commits March 14, 2023 17:30
…b action and wrote unit test

For code coverage added unit test for wrappedNodeFetch function and integrated git hub actions.

test #354

Signed-off-by: Hermione Dadheech <[email protected]>
Revert version and removing console from unit test file

test #354

Signed-off-by: Hermione Dadheech <[email protected]>
Added expect test for mocks and deps after fetching in record and test mode

test #354

Signed-off-by: Hermione Dadheech <[email protected]>
… output with response

Added a check to compare the recorded output with response to ensure wrappedNodeFetch functionality

test #354

Signed-off-by: Hermione Dadheech <[email protected]>
importing HTTP from src/keploy.ts

test #354

Signed-off-by: Hermione Dadheech <[email protected]>
…reateExecutionContext

Resolving the failed test cases by using fetch from node-fetch and createExecutionContext

test#354

Signed-off-by: Hermione Dadheech <[email protected]>
updating test cases

test #354

Signed-off-by: Hermione Dadheech <[email protected]>
Hermione2408 and others added 2 commits March 14, 2023 17:51
removing createExecutionContext from octokit

test #354

Signed-off-by: Hermione Dadheech <[email protected]>
@re-Tick
Copy link
Contributor

re-Tick commented Mar 15, 2023

LGTM. Please add this PR link in GSOC task list.

@developer-diganta developer-diganta added the Accepted PR is reviewed and Accepted label Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted PR is reviewed and Accepted Don't Merge Not to be merged until gsoc results
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[test]: add unit tests for octokit module
3 participants