Move compose e2e tests into pkg #1847
Conversation
ndeloof
force-pushed
the
compose_e2e
branch
2 times, most recently
from
2f36978 to
08d19de
Compare
ndeloof
force-pushed
the
compose_e2e
branch
8 times, most recently
from
c5c9c56 to
1b5bfb8
Compare
Signed-off-by: Nicolas De Loof <[email protected]>
| } | ||
| } | ||
|
|
||
| return &E2eCLI{binDir, d, t} |
There was a problem hiding this comment.
Are we ignoring err if it’s different from «is not exist»?
| } | ||
|
|
||
| func dirContents(dir string) []string { | ||
| res := []string{} |
There was a problem hiding this comment.
nit:
| res := []string{} | |
| var res []string |
| _ = os.RemoveAll(d) | ||
| }) | ||
|
|
||
| _ = os.MkdirAll(filepath.Join(d, "cli-plugins"), 0755) |
|
|
||
| func findExecutable(executableName string, paths []string) (string, error) { | ||
| for _, p := range paths { | ||
| bin, err := filepath.Abs(path.Join(p, executableName)) |
There was a problem hiding this comment.
It should probably use filepath.Join?
| bin, err := filepath.Abs(path.Join(p, executableName)) | |
| bin, err := filepath.Abs(filepath.Join(p, executableName)) |
| return bin, nil | ||
| } | ||
|
|
||
| return "", errors.Wrap(os.ErrNotExist, "executable not found") |
There was a problem hiding this comment.
nit: adding context text to wrapped errors are suggested to be of the form
| return "", errors.Wrap(os.ErrNotExist, "executable not found") | |
| return "", errors.Wrap(os.ErrNotExist, "looking for executable") |
as there is usually a good chance the error already including something around «not found» and this would just add some repetition
| }) | ||
|
|
||
| t.Run("do not display if scan already invoked", func(t *testing.T) { | ||
| _ = os.MkdirAll(filepath.Join(c.ConfigDir, "scan"), 0755) |
There was a problem hiding this comment.
Any reason for ignoring the error here?
| err = CopyFile(composePlugin, filepath.Join(d, "cli-plugins", composePluginFile)) | ||
| if err != nil { |
There was a problem hiding this comment.
nit:
| err = CopyFile(composePlugin, filepath.Join(d, "cli-plugins", composePluginFile)) | |
| if err != nil { | |
| if err := CopyFile(composePlugin, filepath.Join(d, "cli-plugins", composePluginFile)); err != nil { |
| DockerExecutableName = "docker" | ||
| ) | ||
|
|
||
| func init() { | ||
| if runtime.GOOS == "windows" { | ||
| DockerExecutableName = DockerExecutableName + ".exe" | ||
| } | ||
| } |
There was a problem hiding this comment.
Maybe:
| DockerExecutableName = "docker" | |
| ) | |
| func init() { | |
| if runtime.GOOS == "windows" { | |
| DockerExecutableName = DockerExecutableName + ".exe" | |
| } | |
| } | |
| DockerExecutableName = "docker" + binExt() | |
| ) | |
| func binExt() string { | |
| if runtime.GOOS == "windows" { | |
| return ".exe" | |
| } | |
| return "" | |
| } |
It can be re-used further down.
| composePluginFile += ".exe" | ||
| scanPluginFile += ".exe" | ||
| } | ||
| composePlugin, err := findExecutable(composePluginFile, []string{"../../bin", "../../../bin"}) |
There was a problem hiding this comment.
nit: could be
| composePlugin, err := findExecutable(composePluginFile, []string{"../../bin", "../../../bin"}) | |
| composePlugin, err := findExecutable(composePluginFile, "../../bin", "../../../bin") |
with
func findExecutable(executableName string, paths ...string) (string, error) {
| _ = filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { | ||
| res = append(res, filepath.Join(dir, path)) |
There was a problem hiding this comment.
Why are we ignoring errors?
|
@mat007 thanks for the review, but the intent here is not to review the existing tests, but to get them moved into compose.v2 |
What I did
moved compose e2e tests into
pkgso all the compose.v2 code is now isolated from compose-cli