Skip to content

Increase code coverage of SSABasedGenericKubernetesResourceMatcher #2781

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Conversation

Donnerbart
Copy link
Contributor

@Donnerbart Donnerbart commented May 2, 2025

Before: 91% methods, 86% lines covered
After: 100% methods, 99% lines covered

This should be the only missing line now:

if (log.isTraceEnabled()) {
  log.trace("Pruned actual resource:\n {} \ndesired resource:\n {} ", actualYaml, desiredYaml);
}

I hope you don't mind the re-ordering of the methods, but I find code always very hard to read if the methods are not sorted by their invocation order. The only changes to the production class are:

  • Reorder fields and methods.
  • Make getDiff(), sortMap() and sortListItems() static.
  • Reformat code comments.
  • Make keepOnlyManagedFields() package private (to make it directly testable).

@openshift-ci openshift-ci bot requested review from csviri and xstefank May 2, 2025 09:03
Copy link
Collaborator

@xstefank xstefank left a comment

Choose a reason for hiding this comment

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

LGTM

@csviri csviri requested a review from metacosm May 8, 2025 07:48
@csviri
Copy link
Collaborator

csviri commented May 12, 2025

@Donnerbart @metacosm could you pls resolve the conflicts, after would like to merge this. thx

Signed-off-by: David Sondermann <david.sondermann@hivemq.com>
Signed-off-by: David Sondermann <david.sondermann@hivemq.com>
@Donnerbart Donnerbart force-pushed the improvement/increase-code-coverage-of-ssa-matcher branch from 33f1f4e to 1abaea2 Compare May 12, 2025 13:36
@metacosm metacosm merged commit 681dd59 into operator-framework:main May 12, 2025
25 checks passed
@Donnerbart Donnerbart deleted the improvement/increase-code-coverage-of-ssa-matcher branch May 16, 2025 14:08
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants