From d451fa03fcb19e38e9141a4a322bcdf328eeb4e8 Mon Sep 17 00:00:00 2001 From: Suhas Karanth Date: Wed, 20 Sep 2017 11:30:34 +0530 Subject: [PATCH] rename func, improve comments - `project.go` - Rename method {checkCfgFilenames => checkGopkgFilenames} - Improve funciton comment as suggested by @sdboyer - Fix ambigious comment explaining rationale behind early return. - Add comment explaining why we do not use `fs.IsCaseSensitiveFilesystem` for skipping following tests: - context_test.go#TestLoadProjectGopkgFilenames - project_test.go#TestCheckGopkgFilenames - fs_test.go#TestReadActualFilenames --- context.go | 2 +- context_test.go | 10 ++++++++-- internal/fs/fs.go | 2 +- internal/fs/fs_test.go | 9 ++++++++- project.go | 12 +++++++----- project_test.go | 12 +++++++++--- 6 files changed, 34 insertions(+), 13 deletions(-) diff --git a/context.go b/context.go index 6810777adf..f0a80473ab 100644 --- a/context.go +++ b/context.go @@ -105,7 +105,7 @@ func (c *Ctx) LoadProject() (*Project, error) { return nil, err } - err = checkCfgFilenames(root) + err = checkGopkgFilenames(root) if err != nil { return nil, err } diff --git a/context_test.go b/context_test.go index 9f01a9f940..7e5eda6e25 100644 --- a/context_test.go +++ b/context_test.go @@ -264,14 +264,20 @@ func TestLoadProjectNoSrcDir(t *testing.T) { } } -func TestLoadProjectCfgFileCase(t *testing.T) { +func TestLoadProjectGopkgFilenames(t *testing.T) { + // We are trying to skip this test on file systems which are case-sensiive. We could + // have used `fs.IsCaseSensitiveFilesystem` for this check. However, the code we are + // testing also relies on `fs.IsCaseSensitiveFilesystem`. So a bug in + // `fs.IsCaseSensitiveFilesystem` could prevent this test from being run. This is the + // only scenario where we prefer the OS heuristic over doing the actual work of + // validating filesystem case sensitivity via `fs.IsCaseSensitiveFilesystem`. if runtime.GOOS != "windows" && runtime.GOOS != "darwin" { t.Skip("skip this test on non-Windows, non-macOS") } // Here we test that a manifest filename with incorrect case throws an error. Similar // error will also be thrown for the lock file as well which has been tested in - // `project_test.go#TestCheckCfgFilenames`. So not repeating here. + // `project_test.go#TestCheckGopkgFilenames`. So not repeating here. h := test.NewHelper(t) defer h.Cleanup() diff --git a/internal/fs/fs.go b/internal/fs/fs.go index 40695c2d2a..bfacfd95af 100644 --- a/internal/fs/fs.go +++ b/internal/fs/fs.go @@ -272,7 +272,7 @@ var errPathNotDir = errors.New("given path is not a directory") // `os.Stat` and return a map with key and value as filenames which exist in the folder. // // Otherwise, it reads the contents of the directory and returns a map which has the -// given file name as the key and actual filename as the value if it was found. +// given file name as the key and actual filename as the value(if it was found). func ReadActualFilenames(dirPath string, names []string) (map[string]string, error) { actualFilenames := make(map[string]string, len(names)) if len(names) == 0 { diff --git a/internal/fs/fs_test.go b/internal/fs/fs_test.go index f4e26bfb74..4b9422dd88 100644 --- a/internal/fs/fs_test.go +++ b/internal/fs/fs_test.go @@ -265,6 +265,12 @@ func TestIsCaseSensitiveFilesystem(t *testing.T) { } func TestReadActualFilenames(t *testing.T) { + // We are trying to skip this test on file systems which are case-sensiive. We could + // have used `fs.IsCaseSensitiveFilesystem` for this check. However, the code we are + // testing also relies on `fs.IsCaseSensitiveFilesystem`. So a bug in + // `fs.IsCaseSensitiveFilesystem` could prevent this test from being run. This is the + // only scenario where we prefer the OS heuristic over doing the actual work of + // validating filesystem case sensitivity via `fs.IsCaseSensitiveFilesystem`. if runtime.GOOS != "windows" && runtime.GOOS != "darwin" { t.Skip("skip this test on non-Windows, non-macOS") } @@ -332,7 +338,8 @@ func TestReadActualFilenames(t *testing.T) { t.Fatalf("unexpected error: %+v", err) } if !reflect.DeepEqual(c.want, got) { - t.Fatalf("returned value does not match expected: \n\t(GOT) %v\n\t(WNT) %v", got, c.want) + t.Fatalf("returned value does not match expected: \n\t(GOT) %v\n\t(WNT) %v", + got, c.want) } } } diff --git a/project.go b/project.go index 48a554332d..9749a723bf 100644 --- a/project.go +++ b/project.go @@ -42,17 +42,19 @@ func findProjectRoot(from string) (string, error) { } } -// checkCfgFilenames validates filename case for the manifest and lock files. +// checkGopkgFilenames validates filename case for the manifest and lock files. // -// This is relevant on case-insensitive file systems like Windows and macOS. +// This is relevant on case-insensitive file systems like the defaults in Windows and +// macOS. // // If manifest file is not found, it returns an error indicating the project could not be // found. If it is found but the case does not match, an error is returned. If a lock // file is not found, no error is returned as lock file is optional. If it is found but // the case does not match, an error is returned. -func checkCfgFilenames(projectRoot string) error { - // ReadActualFilenames is actually costly. Since this check is not relevant to - // case-sensitive filesystems like ext4(linux), try for an early return. +func checkGopkgFilenames(projectRoot string) error { + // ReadActualFilenames is actually costly. Since the check to validate filename case + // for Gopkg filenames is not relevant to case-sensitive filesystems like + // ext4(linux), try for an early return. caseSensitive, err := fs.IsCaseSensitiveFilesystem(projectRoot) if err != nil { return errors.Wrap(err, "could not check validity of configuration filenames") diff --git a/project_test.go b/project_test.go index c76702d03a..c3d1ef00ba 100644 --- a/project_test.go +++ b/project_test.go @@ -63,7 +63,13 @@ func TestFindRoot(t *testing.T) { } } -func TestCheckCfgFilenames(t *testing.T) { +func TestCheckGopkgFilenames(t *testing.T) { + // We are trying to skip this test on file systems which are case-sensiive. We could + // have used `fs.IsCaseSensitiveFilesystem` for this check. However, the code we are + // testing also relies on `fs.IsCaseSensitiveFilesystem`. So a bug in + // `fs.IsCaseSensitiveFilesystem` could prevent this test from being run. This is the + // only scenario where we prefer the OS heuristic over doing the actual work of + // validating filesystem case sensitivity via `fs.IsCaseSensitiveFilesystem`. if runtime.GOOS != "windows" && runtime.GOOS != "darwin" { t.Skip("skip this test on non-Windows, non-macOS") } @@ -111,11 +117,11 @@ func TestCheckCfgFilenames(t *testing.T) { tmpPath := h.Path(".") // Create any files that are needed for the test before invoking - // `checkCfgFilenames`. + // `checkGopkgFilenames`. for _, file := range c.createFiles { h.TempFile(file, "") } - err := checkCfgFilenames(tmpPath) + err := checkGopkgFilenames(tmpPath) if c.wantErr { if err == nil {