From 2e3700786d702a1f6c687e23f2916e8ae929d723 Mon Sep 17 00:00:00 2001 From: xdrm-brackets Date: Wed, 14 Nov 2018 22:10:31 +0100 Subject: [PATCH] fix/upd instruction/copy | add tests --- internal/instruction/copy.go | 22 ++- internal/instruction/copy_test.go | 308 ++++++++++++++++++++++++++++++ internal/instruction/errors.go | 21 +- 3 files changed, 337 insertions(+), 14 deletions(-) create mode 100644 internal/instruction/copy_test.go diff --git a/internal/instruction/copy.go b/internal/instruction/copy.go index 836a1b8..03e71e4 100644 --- a/internal/instruction/copy.go +++ b/internal/instruction/copy.go @@ -1,7 +1,6 @@ package instruction import ( - "fmt" "os" "strings" ) @@ -17,7 +16,7 @@ func (d *copy) Raw() string { return strings.Join([]string{"copy", d.raw}, " ") func (d *copy) Build(_args string) error { // 1. extract action (sub command) - split := strings.Split(_args, " ") + split := strings.Fields(_args) // 2. check syntax if len(split) != 2 { @@ -34,12 +33,12 @@ func (d copy) Exec(ctx ExecutionContext) ([]byte, error) { // 1. fail if source file not found if _, err := os.Stat(d.Src); os.IsNotExist(err) { - return nil, fmt.Errorf("cannot find script '%s'", d.Src) + return nil, &FileError{"cannot find file", d.Src, err} } // 2. execute script - if err := ctx.Executor.Command("cp", d.Src, d.Dst).Run(); err != nil { - return nil, fmt.Errorf("cannot copy '%s' to '%s' | %s", d.Src, d.Dst, err) + if err := ctx.Executor.Command("cp", "-r", d.Src, d.Dst).Run(); err != nil { + return nil, &FileError{"cannot copy to", d.Dst, err} } return nil, nil @@ -50,30 +49,33 @@ func (d copy) DryRun(ctx ExecutionContext) ([]byte, error) { // 1. fail if source file not found if _, err := os.Stat(d.Src); os.IsNotExist(err) { - return nil, fmt.Errorf("cannot find file '%s'", d.Src) + return nil, &FileError{"cannot find file", d.Src, err} } // 2. if destination to create : try to create (then remove) - if _, err := os.Stat(d.Dst); os.IsNotExist(err) { + fi, err := os.Stat(d.Dst) + if os.IsNotExist(err) { file, err2 := os.OpenFile(d.Dst, os.O_APPEND|os.O_WRONLY|os.O_CREATE, os.FileMode(0777)) if err2 != nil { - return nil, fmt.Errorf("cannot copy to '%s' | %s", d.Dst, err2) + return nil, &FileError{"cannot copy to", d.Dst, err2} } file.Close() if err2 := os.Remove(d.Dst); err2 != nil { - return nil, fmt.Errorf("cannot remove dry-run file '%s' | %s", d.Dst, err2) + return nil, &FileError{"cannot remove dry-run file", d.Dst, err2} } return nil, nil + } else if fi != nil && fi.IsDir() { + return nil, nil // no error if dir } // 3. if destination exists : check write permission file, err := os.OpenFile(d.Dst, os.O_APPEND|os.O_WRONLY, 0600) if err != nil { - return nil, fmt.Errorf("cannot copy to '%s' | %s", d.Dst, err) + return nil, &FileError{"cannot copy to", d.Dst, err} } file.Close() diff --git a/internal/instruction/copy_test.go b/internal/instruction/copy_test.go new file mode 100644 index 0000000..6dfb83d --- /dev/null +++ b/internal/instruction/copy_test.go @@ -0,0 +1,308 @@ +package instruction + +import ( + "fmt" + "os" + "testing" +) + +func TestCopyInvalidSyntax(t *testing.T) { + tests := []string{ + "one-arg", + "src dst extra-arg", + } + + for i, test := range tests { + inst := new(copy) + err := inst.Build(test) + if err != ErrInvalidSyntax { + t.Errorf("[%d] expected error <%s>, got <%s>", i, ErrInvalidSyntax, err) + } + } +} +func TestCopyBuildArgs(t *testing.T) { + tests := []string{ + "source destination", + "\tsource destination", + " source destination", + "source\t destination", + "source destination", + "source \tdestination", + "source destination", + "source destination\t", + "source\t\tdestination\t", + "source \t\t destination\t", + "source\t \tdestination\t", + } + + for i, test := range tests { + inst := new(copy) + err := inst.Build(test) + if err != nil { + t.Errorf("[%d] unexpected error <%s>", i, err) + continue + } + + if inst.Src != "source" { + t.Errorf("[%d] expected 'source', got '%s'", i, inst.Src) + continue + } + if inst.Dst != "destination" { + t.Errorf("[%d] expected 'source', got '%s'", i, inst.Dst) + continue + } + + } +} + +func TestCopySourceNotExist(t *testing.T) { + defer os.RemoveAll("/tmp/destination") + raw := "/tmp/source /tmp/destination" + + ctx, err := CreateContext("apt-get", "") + if err != nil { + t.Fatalf("cannot create context") + } + + for i := 0; i < 2; i++ { + inst := new(copy) + err := inst.Build(raw) + if err != nil { + t.Fatalf("[%d] unexpected error <%s>", i, err) + } + + if i == 0 { + _, err = inst.Exec(*ctx) + } else { + _, err = inst.DryRun(*ctx) + } + if err == nil { + t.Fatalf("[%d] expected error", i) + } + + ce, ok := err.(*FileError) + if !ok { + t.Fatalf("[%d] expected error of type <*FileError>", i) + } + if ce.Reason != "cannot find file" || ce.File != "/tmp/source" { + t.Fatalf("[%d] expected error <%s '%s'> got <%s '%s'>", i, "cannot find file", "/tmp/source", ce.Reason, ce.File) + } + } +} + +func TestCopySourceIsDir(t *testing.T) { + src, dst := "/tmp/sourcedir", "/tmp/destinationdir" + raw := fmt.Sprintf("%s %s", src, dst) + defer os.RemoveAll(src) + defer os.RemoveAll(dst) + + ctx, err := CreateContext("apt-get", "") + if err != nil { + t.Fatalf("cannot create context") + } + + for i := 0; i < 2; i++ { + // 1. create directory + os.RemoveAll(src) + if err := os.MkdirAll(src, os.FileMode(0777)); err != nil { + t.Fatalf("[%d] cannot create test directory | %s", i, err) + } + + inst := new(copy) + err := inst.Build(raw) + if err != nil { + t.Fatalf("[%d] unexpected error <%s>", i, err) + } + + if i == 0 { + _, err = inst.Exec(*ctx) + } else { + _, err = inst.DryRun(*ctx) + } + if err != nil { + t.Fatalf("[%d] unexpected error <%s>", i, err) + } + } + +} +func TestCopyInvalidDestination(t *testing.T) { + src, dst := "/tmp/source", "/tmp/missing-directory/invalid-destination" + raw := fmt.Sprintf("%s %s", src, dst) + defer os.RemoveAll(src) + defer os.RemoveAll(dst) + + ctx, err := CreateContext("apt-get", "") + if err != nil { + t.Fatalf("cannot create context") + } + + for i := 0; i < 2; i++ { + + os.RemoveAll(src) + // 1. create directory + if err := os.MkdirAll(src, os.FileMode(0777)); err != nil { + t.Fatalf("[%d] cannot create test directory | %s", i, err) + } + inst := new(copy) + err := inst.Build(raw) + if err != nil { + t.Fatalf("[%d] unexpected error <%s>", i, err) + } + + if i == 0 { + _, err = inst.Exec(*ctx) + } else { + _, err = inst.DryRun(*ctx) + } + if err == nil { + t.Fatalf("[%d] expected error", i) + } + + ce, ok := err.(*FileError) + if !ok { + t.Fatalf("[%d] expected error of type <*FileError>", i) + } + if ce.Reason != "cannot copy to" || ce.File != dst { + t.Fatalf("[%d] expected error <%s '%s'> got <%s '%s'>", i, "cannot copy to", dst, ce.Reason, ce.File) + } + + } +} +func TestCopySourceIsFile(t *testing.T) { + src, dst := "/tmp/source", "/tmp/destination-file" + raw := fmt.Sprintf("%s %s", src, dst) + defer os.RemoveAll(src) + defer os.RemoveAll(dst) + + ctx, err := CreateContext("apt-get", "") + if err != nil { + t.Fatalf("cannot create context") + } + + for i := 0; i < 2; i++ { + + os.RemoveAll(src) + + // 1. create directory + fd, err := os.Create(src) + if err != nil { + t.Fatalf("[%d] cannot create test file | %s", i, err) + } + fd.Close() + + inst := new(copy) + err = inst.Build(raw) + if err != nil { + t.Fatalf("[%d] unexpected error <%s>", i, err) + } + + if i == 0 { + _, err = inst.Exec(*ctx) + } else { + _, err = inst.DryRun(*ctx) + } + if err != nil { + t.Fatalf("[%d] unexpected error <%s>", i, err) + } + + } +} + +func TestCopySourceIsFileDestinationExists(t *testing.T) { + src, dst := "/tmp/source", "/tmp/destination-file" + raw := fmt.Sprintf("%s %s", src, dst) + defer os.RemoveAll(src) + defer os.RemoveAll(dst) + + ctx, err := CreateContext("apt-get", "") + if err != nil { + t.Fatalf("cannot create context") + } + + for i := 0; i < 2; i++ { + + os.RemoveAll(src) + os.RemoveAll(dst) + + // 1. create source + fd, err := os.Create(src) + if err != nil { + t.Fatalf("[%d] cannot create test file | %s", i, err) + } + fd.Close() + // 1. create destination + fd, err = os.Create(src) + if err != nil { + t.Fatalf("[%d] cannot create test file | %s", i, err) + } + fd.Close() + + inst := new(copy) + err = inst.Build(raw) + if err != nil { + t.Fatalf("[%d] unexpected error <%s>", i, err) + } + + if i == 0 { + _, err = inst.Exec(*ctx) + } else { + _, err = inst.DryRun(*ctx) + } + if err != nil { + t.Fatalf("[%d] unexpected error <%s>", i, err) + } + + } +} +func TestCopyCannotWriteDestination(t *testing.T) { + src, dst := "/tmp/source", "/tmp/destination-perm" + raw := fmt.Sprintf("%s %s", src, dst) + defer os.RemoveAll(src) + defer os.RemoveAll(dst) + + ctx, err := CreateContext("apt-get", "") + if err != nil { + t.Fatalf("cannot create context") + } + + for i := 0; i < 2; i++ { + + os.RemoveAll(src) + if err := os.RemoveAll(dst); err != nil { + t.Fatalf("[%d] cannot remove destination file\n", i) + } + + // 1. create source + fd, err := os.Create(src) + if err != nil { + t.Fatalf("[%d] cannot create test file | %s", i, err) + } + fd.Close() + // 1. create destination + fd, err = os.Create(src) + if err != nil { + t.Fatalf("[%d] cannot create test file | %s", i, err) + } + err = fd.Chmod(os.FileMode(0555)) + if err != nil { + t.Fatalf("[%d] cannot set permissions | %s", i, err) + } + fd.Close() + + inst := new(copy) + err = inst.Build(raw) + if err != nil { + t.Fatalf("[%d] unexpected error <%s>", i, err) + } + + if i == 0 { + _, err = inst.Exec(*ctx) + } else { + _, err = inst.DryRun(*ctx) + } + if err != nil { + t.Fatalf("[%d] unexpected error <%s>", i, err) + } + + } +} diff --git a/internal/instruction/errors.go b/internal/instruction/errors.go index 2448798..3ad5cc4 100644 --- a/internal/instruction/errors.go +++ b/internal/instruction/errors.go @@ -1,15 +1,28 @@ package instruction import ( - "errors" + "fmt" ) // ErrInvalidAlias is raised when encountering an invalid token in an alias name -var ErrInvalidAlias = errors.New("invalid alias name (contains '/')") +var ErrInvalidAlias = fmt.Errorf("invalid alias name (contains '/')") // ErrInvalidSyntax is raised when encountering an invalid token -var ErrInvalidSyntax = errors.New("invalid instruction format") +var ErrInvalidSyntax = fmt.Errorf("invalid instruction format") // ErrUnknownInstruction is raised when encountering an unknown instruction // it can mean that you're not using the right version or that you've misspelled it -var ErrUnknownInstruction = errors.New("unknown instruction") +var ErrUnknownInstruction = fmt.Errorf("unknown instruction") + +type FileError struct { + Reason string + File string + Err error +} + +func (ce FileError) Error() string { + if ce.Err == nil { + return fmt.Sprintf("%s '%s'", ce.Reason, ce.File) + } + return fmt.Sprintf("%s '%s' | %s", ce.Reason, ce.File, ce.Err) +}