Migrates cmd tests from ginkgo,gomega to testing,testify #161
Conversation
This file contains hidden or 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
codefromthecrypt
commented
Apr 4, 2021
| @@ -250,8 +210,8 @@ func requireExtensionClean(t *testing.T, workDir string) { | |||
| err := os.Chdir(workDir) | |||
| require.NoError(t, err, `error changing to directory: %v`, workDir) | |||
|
|
|||
| cmd := getEnvoy("extension clean") | |||
There was a problem hiding this comment.
these cmd -> c changes were mainly to help avoid clashing with the package
codefromthecrypt
commented
Apr 4, 2021
|
|
||
| // Package morerequire includes more require functions than "github.com/stretchr/testify/require" | ||
| // Do not add dependencies on any main code as it will cause cycles. | ||
| package morerequire |
There was a problem hiding this comment.
the stuff here is the same as it was, this is just pulled out to help reduce chance of package cycles
mathetake
reviewed
Apr 6, 2021
added 3 commits
April 6, 2021 09:16
This completes migration of ./pkg/cmd/... from ginkgo,gomega to testing,testify per rationale noted in #130. This also fixes a bug where we accidentally added ignore files into the output directory of `getenvoy extension init` and backfills tests to ensure that doesn't happen again. Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
Before, we added `.gitignore` files on `git extension init`. These assume the end-user would be checking the workspaces into git. However, the workspaces were neither initialized for git, nor the `.gitignore` files complete per-language. Moreover, certain directories are subject to variables, such as `CARGO_TARGET_DIR`. This removes the .gitignore file generation completely, and with it the duplication involved. If an end user is using our project, able to do `git init`, yet want us to manage `.gitignore` also, we can revisit upon such feedback later. Signed-off-by: Adrian Cole <[email protected]>
codefromthecrypt
force-pushed
the
complete-cmd-migration
branch
from
7aa5fa6 to
010f67b
Compare
Signed-off-by: Adrian Cole <[email protected]>
mathetake
reviewed
Apr 6, 2021
mathetake
reviewed
Apr 6, 2021
mathetake
reviewed
Apr 6, 2021
| func() []TableEntry { | ||
| entries := []TableEntry{} | ||
| for _, category := range []string{"envoy.filters.http", "envoy.filters.network", "envoy.access_loggers"} { | ||
| for _, language := range []string{"rust"} { |
There was a problem hiding this comment.
oops, I didn't know that this is only tested for Rust, thanks
mathetake
reviewed
Apr 6, 2021
| err = newParams().OutputDir.Validator(invalidPath) | ||
| Expect(err).To(MatchError(fmt.Sprintf(`stat %s: not a directory`, invalidPath))) | ||
| }) | ||
| file := os.Args[0] |
|
thanks! will address the feedback shortly |
Signed-off-by: Adrian Cole <[email protected]>
|
Thanks for another epic review @mathetake |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This completes migration of ./pkg/cmd/... from ginkgo,gomega to
testing,testify per rationale noted in #130.
This also fixes a bug where we accidentally added ignore files into
the output directory of
getenvoy extension initand backfills teststo ensure that doesn't happen again.
elaboration on .gitignore and
getenvoy extension initBefore, we added
.gitignorefiles ongetenvoy extension init. Theseassume the end-user would be checking the workspaces into git. However,
the workspaces were neither initialized for git, nor the
.gitignorefiles complete per-language. Moreover, certain directories are subject
to variables, such as
CARGO_TARGET_DIR.This removes the .gitignore file generation completely, and with it the
duplication involved. If an end user is using our project, able to do
git init, yet want us to manage.gitignorealso, we can revisit uponsuch feedback later.