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

Added table-driven tests with updated scenarios for ResultV2 #682

Closed
wants to merge 3 commits into from

Conversation

zhaque44
Copy link
Contributor

The ResultV2 struct has the following (AddChange, Changes, Objects) in the update pkg do not have test coverage.

TestResultV2_AddChange: Tests the AddChange method to ensure that changes are correctly added to the result. It creates a new ResultV2, adds a change using AddChange method, and checks if the change was added correctly.

TestResultV2_Changes: Tests the Changes method to ensure that changes are returned correctly. It sets the FileChanges field of the ResultV2 struct, calls the Changes method, and verifies if the changes are returned.

TestResultV2_Objects: Tests the Objects method to ensure that objects are returned correctly. It sets the FileChanges field of the ResultV2 struct, calls the Objects method, and verifies if the objects are returned.

Signed-off-by: zhaque44 <haque.zubair@gmail.com>
@zhaque44
Copy link
Contributor Author

@stefanprodan can you please take a look when you have a minute

@souleb
Copy link
Member

souleb commented May 17, 2024

thanks for the PR @zhaque44 but I don't really see why we should add it. Those functions are already tested in the existing test TestResultV2.

@zhaque44
Copy link
Contributor Author

zhaque44 commented May 17, 2024

thanks for the PR @zhaque44 but I don't really see why we should add it. Those functions are already tested in the existing test TestResultV2.

Hi @souleb hope you are well very nice to meet you the original test is missing:

Testing AddChange Method <- this is not in the original test

Also, the test I have written focuses on more comprehensive assertions: Each subtest in the table driven test function focuses on ensuring different aspects of ResultV2 are covered.

@zhaque44
Copy link
Contributor Author

@stefanprodan @souleb can we get a final review please, the assertions I’m adding bring value, if the team doesn’t think so I’ll close it. Please review the original test and what I have added. Thanks

@souleb
Copy link
Member

souleb commented May 21, 2024

the only difference I see is that you add an assertion for AddChange where the original code does not. We could add that.

Other than that, the test cases presented are the same.

Signed-off-by: zhaque44 <haque.zubair@gmail.com>
@zhaque44
Copy link
Contributor Author

@souleb my branch is in a bad state, will open a new PR with the changes:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants