strimzi-kafka-operator
strimzi-kafka-operator copied to clipboard
strimzi
refactor Metric assertions in TopicOperatorTests
Type of change
- Refactoring
Description
Reviewing https://github.com/strimzi/strimzi-kafka-operator/pull/7338 I noticed a lot of repeated code about asserting values of various metrics. In each block asserting metrics there is a lot of "noise" around finding the appropriate metrics in the repository and then matching the value.
So this change pulls out some custom assertion methods so that its easier to see what each assertion is trying to prove about the metrics.
It might be judicious to wait for #7338 to be merged before merging this change as that ensures all the tests actually pass before refactoring them. However I based the PR on main as there is no direct dependency between them.
Checklist
Please go through this checklist and make sure all applicable tasks have been done
- [X] Write tests
- [X] Make sure all tests pass
- [ ] Update documentation
- [ ] Check RBAC rights for Kubernetes / OpenShift roles
- [ ] Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally
- [ ] Reference relevant issue(s) and close them after merging
- [ ] Update CHANGELOG.md
- [ ] Supply screenshots for visual changes, such as Grafana dashboards
The changes look good to me, but in our investigations into the failing tests under PR #7338 we noticed some problems with the metric assertions, for example tests asserting metric results where the code that drives those metrics isn't being exercised. So I think it would probably make sense to hold off on this change until we have resolved those problems. Also until that is merged this change is refactoring code that often isn't being exercised as many tests are completing early (which is what 7338 is addressing) so another reason to hold off until we can see that these changes don't break the tests for certain.
