Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

Commit

Permalink
rename func, improve comments
Browse files Browse the repository at this point in the history
- `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
  • Loading branch information
sudo-suhas committed Sep 20, 2017
1 parent 5b2e0d0 commit 277ba75
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 13 deletions.
2 changes: 1 addition & 1 deletion context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
10 changes: 8 additions & 2 deletions context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion internal/fs/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
9 changes: 8 additions & 1 deletion internal/fs/fs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down Expand Up @@ -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)
}
}
}
Expand Down
12 changes: 7 additions & 5 deletions project.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
12 changes: 9 additions & 3 deletions project_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 277ba75

Please sign in to comment.