This repository has been archived by the owner on Jun 29, 2022. It is now read-only.
pkg/terraform: improvements #1027
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
invidian
force-pushed
the
invidian/terraform-errors-improvements
branch
from
0479ff6 to
78abf0a
Compare
Also use t.Cleanup() instead of defer to cleanup after tests. Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
So regular unit tests for executor can be run. Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
Rather than in runtime. Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
invidian
force-pushed
the
invidian/terraform-errors-improvements
branch
from
78abf0a to
300075b
Compare
surajssd
reviewed
Sep 30, 2020
surajssd
reviewed
Sep 30, 2020
| return fmt.Errorf("checking Terraform version: %w", err) | ||
| } | ||
| // requiredVersion is const, so we test it in unit tests. | ||
| constraints, _ := version.NewConstraint(requiredVersion) |
There was a problem hiding this comment.
we can log it in debug mode? So we don't lose track of it altogether.
There was a problem hiding this comment.
Why? It is covered by unit tests, running exactly the same code.
There was a problem hiding this comment.
It's best to panic in such case, if this error should be found during development itself.
surajssd
reviewed
Sep 30, 2020
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| //+build e2e |
There was a problem hiding this comment.
Should e2e tests sit in test dir? Mixing unit tests and e2e tests?
There was a problem hiding this comment.
IMO, those are package-scoped tests, so I think it's fine if they sit in this directory. They are marked as e2e, as they require external dependency to pass (terraform binary).
knrt10
reviewed
Oct 5, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of #1023.