test(utils): fix tests that assert nothing and run them in CI - #1680
test(utils): fix tests that assert nothing and run them in CI#1680ihopenre-eng wants to merge 1 commit into
Conversation
The utils package is not part of `make test`, which runs `./pkg/...`
only, so its tests never ran in CI. Three of them fail today.
TestValidateFilePaths and TestReadFile create a temp fixture and remove
it with `defer` while their subtests call `t.Parallel()`. A parallel
subtest is paused until its parent returns, so the deferred cleanup
deletes the fixture before any subtest reads it, and the "valid file
path" cases assert an error that only proves the file is gone. This
fails on every platform:
--- FAIL: TestValidateFilePaths (0.01s)
--- FAIL: TestReadFile (0.00s)
Move both to t.Cleanup, which runs after parallel subtests finish, the
pattern TestFindFileInDir in this same file already uses.
TestGetAbsPath additionally fails on Windows: "/absolute/path" has no
volume name, so filepath.IsAbs reports false for it and GetAbsPath joins
it with baseDir. Pick the absolute path per platform.
Add ./utils/... to the test target so these stay honest.
Signed-off-by: ihopenre-eng <247072151+ihopenre-eng@users.noreply.github.com>
|
The CI workflows on this PR are queued at Could a maintainer approve the workflow runs when convenient? Happy to rebase if anything has drifted in the meantime. |
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
|
This pull request has been automatically closed because it was marked stale with no further activity for 7 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
Description
The
utilspackage is not part ofmake test, which runs./pkg/...only, so its tests never ran in CI. Three of them fail today on a clean master.TestValidateFilePathsandTestReadFilecreate a temp fixture and remove it withdeferwhile their subtests callt.Parallel(). A parallel subtest is paused until its parent function returns, so the deferred cleanup deletes the fixture before any subtest reads it, and the "valid file path" cases assert an error that only proves the file is already gone. They fail on Linux and Windows alike. Both move tot.Cleanup, which runs after parallel subtests finish, matching whatTestFindFileInDirin the same file already does.TestGetAbsPathfails on Windows only:/absolute/pathcarries no volume name, sofilepath.IsAbsreports false for it andGetAbsPathjoins it withbaseDir. The absolute path is now chosen per platform../utils/...is added to thetesttarget so these keep working.Before:
After, on windows/amd64 and on linux/amd64 (cross-compiled test binary run under WSL):
No production code is touched.
Issue reference
Please reference the issue this PR will close: #1679
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: