search

fluxcd / image-automation-controller

GitOps Toolkit controller that patches container image tags in Git
https://fluxcd.io
Apache License 2.0
155 stars 67 forks source link

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

Closed zhaque44 closed 1 month ago

zhaque44 commented 1 month ago

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.

zhaque44 commented 1 month ago

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

souleb commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

@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 commented 1 month ago

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.

zhaque44 commented 1 month ago

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