(bugfix): crdify generator returns aggregated error when validations fail #2493
Conversation
|
Hello @everettraven! Some important instructions when contributing to openshift/api: |
everettraven
changed the title
openshift-ci
bot
added
the
size/S
| results = append(results, result) | ||
| } | ||
|
|
||
| if errs != nil { |
There was a problem hiding this comment.
Why not check directly against result.Errors? It's a list of errors already so could just check that's non zero and create an aggregate from that no?
There was a problem hiding this comment.
This is after building the list of generator results. We would need to iterate through all the results and their errors instead of building the error list as we go.
That being said, this should actually be
| if errs != nil { | |
| if len(errs) > 0 { |
…n validations fail Signed-off-by: Bryce Palmer <[email protected]>
everettraven
force-pushed
the
bugfix/crdify-exit
branch
from
c708f0a to
7076620
Compare
|
/lgtm Small tooling change, doesn't need a bypass |
openshift-ci-robot
added
the
verified
|
@JoelSpeed: The In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
/test remaining-required Overriding unmatched contexts: |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JoelSpeed The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
@openshift-ci-robot: The specified target(s) for The following commands are available to trigger optional jobs: Use In response to this:
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. |
|
@openshift-ci-robot: Overrode contexts on behalf of openshift-ci-robot: ci/prow/e2e-aws-ovn, ci/prow/e2e-aws-ovn-hypershift, ci/prow/e2e-aws-ovn-hypershift-conformance, ci/prow/e2e-aws-ovn-techpreview, ci/prow/e2e-aws-serial-1of2, ci/prow/e2e-aws-serial-2of2, ci/prow/e2e-aws-serial-techpreview-1of2, ci/prow/e2e-aws-serial-techpreview-2of2, ci/prow/e2e-azure, ci/prow/e2e-gcp, ci/prow/e2e-upgrade, ci/prow/e2e-upgrade-out-of-change In response to this:
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. |
|
@everettraven: all tests passed! Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
We noticed in #2492 that the
crdifyverification passed when we would have expected it to fail.We noticed in the logs that it did properly return failed validation in the logs but still exited successfully.
This was because my original PR to add the
crdifycodegen generator did not count the errors from the comparison validations as an error to be returned and instead we only built out the generator results.This PR updates the method where the comparison validations occur to consider errors in the validations result as a normal Go error as well, returning an aggregated error of all comparison validation errors that have occurred alongside the generator results.