test: fixing issue realted to test indentation #2786
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: nmn3m The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
needs-triage
|
This issue is currently awaiting triage. If kube-state-metrics contributors determine this is a relevant issue, they will accept it by applying the The Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
k8s-ci-robot
added
the
size/XS
|
|
||
| for _, l := range lines { | ||
| trimmedLine = strings.TrimSpace(l) | ||
| trimmedLine = strings.TrimLeft(l, " \t") |
There was a problem hiding this comment.
why do we need to check for tabs as well? Could we only do spaces?
There was a problem hiding this comment.
I went with strings.TrimLeft(l, " \t") instead of just " " because when I only trimmed spaces, a lot of tests failed. Using both spaces and tabs handled the mixed indentation and fixed the test failures.
There was a problem hiding this comment.
Can we fix the indentation instead to only use spaces or won't that work?
There was a problem hiding this comment.
I tried to fix the indentation using the white spaces, and the tests failed.
I think using the combination between white spaces and tab will be a good choice in that case.
What this PR does / why we need it:
This PR fixes an issue where test files contained inconsistent indentation (spaces and tabs) that were not caught by linting.
We now normalize leading whitespace in test parsing by using:
Additionally, some failing tests were updated to align with the corrected logic.
How does this change affect the cardinality of KSM:
does not change cardinality
Which issue(s) this PR fixes:
Fixes #2776