-
Notifications
You must be signed in to change notification settings - Fork 68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TestBuilder(): be able to expect errors #244
Conversation
@@ -775,9 +776,6 @@ func TestBuilder(t *testing.T) { | |||
Image: "centos:7", | |||
Shell: []string{"/bin/bash", "-xc"}, | |||
}, | |||
ErrFn: func(err error) bool { | |||
return err != nil && strings.Contains(err.Error(), "multiple FROM statements are not supported") | |||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why this is getting removed? More tea for me?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Dockerfile the test uses doesn't have multiple FROM statements in it, so the function was never called. Under the new paradigm, having a function that decides whether or not to ignore an error, but not having an error to pass to it, would cause the test to fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, thought this test had multiple FROM calls. SGTM.
LGTM |
5096a53
to
b39c888
Compare
/hold |
Update some test cases so that we can fail a test if we expect to get an error, but don't see one. Remove the expectation of a "no multiple FROMs allowed" error from the Dockerfile.shell test case, which only used one FROM in its Dockerfile. Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
@nalind: 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/test-infra repository. I understand the commands that are listed here. |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: nalind, rhatdan 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 |
/hold cancel |
Update some test cases so that we can fail a test if we expect to get an error, but don't see one.