-
-
Notifications
You must be signed in to change notification settings - Fork 579
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
Don't use err when it's nil #1046
Conversation
b373a16
to
912360a
Compare
pkg/appimport/appimport_test.go
Outdated
assert.Equal(err.Error(), "is archive") | ||
if err != nil { | ||
assert.Equal(err.Error(), "is archive") | ||
} |
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.
We'll want to wait until the import-files refactor PR is in and rebase this as the function signature for ValidateAsset has changed, it now returns a bool isArchive value and doesn't rely on the error value.
912360a
to
9d2e270
Compare
Rebased this, think it does what it should. I did figure out that one test failure - there was a cached download for a tarball that should never have succeeded. Not sure how that happened, but cleaning up ~/.ddev/testcache/ fixed it I think. |
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.
Everything looks good after the rebase, 👍 from me
The Problem/Issue/Bug:
In a couple of our tests we occasionally get panics because we get a nil for err and then use err.Error(). We shouldn't use a nil err.
How this PR Solves The Problem:
This affects only tests, and only occasionally, so is a code-review.
See the fail/panic in https://buildkite.com/drud/ddev-macos/builds/489#6c3c127b-8d24-40f2-861e-9ea7704c07e3