fileutils_test: check symlink directory location before test#603
fileutils_test: check symlink directory location before test#603mtrmac merged 1 commit intocontainers:mainfrom
Conversation
mtrmac
left a comment
There was a problem hiding this comment.
Thanks.
I expect that works, but really tests should not be hard-coding specific paths in /tmp (and exposing the invoking user to attacks by other users on the machine) in the first place. Can you convert the test to be based on t.TempDir(), please? That might work around the macOS /tmp situation as a side effect.
On some systems, /tmp and /var are symlinks to /private/tmp and /private/var, respectively. This may cause the test to fail, because the created directory is not actually at the path where it was created. Get the canonical path of the created directory and compare that to the output of ReadSymlinkedDirectory. While we're here, avoid hardcoding specific test paths and prefer using t.TempDir() instead. This also simplifies the cleanup logic. Signed-off-by: Caleb Xu <caxu@redhat.com>
19562db to
5d306e0
Compare
|
Thanks for the feedback, I made adjustments to avoid using hardcoded paths for the test. The path returned by I wonder if this should work now on Windows too, but I don't have the means to test it right now, so I've left the TODO in place. |
|
If @mtrmac is hip with this, so am I. Provisionally: |
| if err = os.Remove("/tmp/dirLinkTest"); err != nil { | ||
| t.Errorf("failed to remove symlink: %s", err) | ||
| } | ||
| require.Equal(t, path, realpath, "symlink returned unexpected directory: %s", path) |
There was a problem hiding this comment.
(For the record, the argument order is Equal(t, expected, actual). Not worth worrying that much about I guess.)
On some systems, /tmp and /var are symlinks to /private/tmp and /private/var, respectively. This may cause the test to fail, because the created directory is not actually at the path where it was created.
Get the canonical path of the created directory and compare that to the output of ReadSymlinkedDirectory.
While we're here, avoid hardcoding specific test paths and prefer using t.TempDir() instead. This also simplifies the cleanup logic.