Chore: spurious helpers test cleanup #3729
Conversation
… tests Signed-off-by: apostasie <[email protected]>
| buildCtx := data.TempDir() | ||
| err := os.WriteFile(filepath.Join(buildCtx, "Dockerfile"), []byte(dockerfile), 0o600) | ||
| assert.NilError(helpers.T(), err) |
There was a problem hiding this comment.
why we cannot use the helper directly?
There was a problem hiding this comment.
The helper is misplaced under /cmd (like a bunch of others that should also be moved to pkg/testing or removed). It is calling t.TempDir() every time it is being called, creating a new separate temporary directory at each invocation, and buries trivial logic away from the test - for the benefit of replacing 3 lines for 1.
I do not think it is worth it. If folks here prefer to keep a helper, we should copy it over to pkg/test and change the signature of the function to at least allow passing data and helpers so that is uses data.TempDir and helpers.T instead.
There was a problem hiding this comment.
(I say "copy it over" because a bunch of legacy tests are still dependent on the existing helper and would not be able to use the new one until rewritten)
|
Thanks @djdongjin Appreciate the review. |
Remove dependency on misplaced helper CreateBuildContext for next-gen tests (use data.TempDir(), assert in the test).