Increase umask code coverage from 62% to 100%

Related #1062

pkg/eval/errs/errs.go

@@ -12,6 +12,7 @@ type OutOfRange struct {
 	ValidLow  int
 	ValidHigh int
 	Actual    string
+	ValidFmt  string
 }
 
 func (e OutOfRange) Error() string {
@@ -19,8 +20,12 @@ func (e OutOfRange) Error() string {
 		return fmt.Sprintf(
 			"out of range: %v has no valid value, but is %v", e.What, e.Actual)
 	}
+	validFmt := e.ValidFmt
+	if validFmt == "" {
+		validFmt = "%d"
+	}
 	return fmt.Sprintf(
-		"out of range: %v must be from %v to %v, but is %v",
+		"out of range: %s must be from "+validFmt+" to "+validFmt+", but is %s",
 		e.What, e.ValidLow, e.ValidHigh, e.Actual)
 }
 

pkg/eval/unix/umask.go

@@ -3,7 +3,6 @@
 package unix
 
 import (
-	"errors"
 	"fmt"
 	"math"
 	"strconv"
@@ -11,9 +10,15 @@ import (
 
 	"golang.org/x/sys/unix"
 
+	"github.com/elves/elvish/pkg/eval/errs"
+	"github.com/elves/elvish/pkg/eval/vals"
 	"github.com/elves/elvish/pkg/eval/vars"
 )
 
+const (
+	validUmaskMsg = "integer in the range [0..0o777]"
+)
+
 //elvdoc:var umask
 //
 // The file mode creation mask. Its value is a string in Elvish octal
@@ -70,22 +75,29 @@ func (UmaskVariable) Set(v interface{}) error {
 		if err != nil {
 			i, err = strconv.ParseInt(v, 0, 0)
 			if err != nil {
-				return errors.New("umask value not a valid number")
+				return errs.BadValue{
+					What: "umask", Valid: validUmaskMsg, Actual: vals.ToString(v)}
 			}
 		}
 		umask = int(i)
 	case float64:
 		intPart, fracPart := math.Modf(v)
 		if fracPart != 0 {
-			return errors.New("umask value must be an integer")
+			return errs.BadValue{
+				What: "umask", Valid: validUmaskMsg, Actual: vals.ToString(v)}
 		}
 		umask = int(intPart)
 	default:
-		return errors.New("umask value must be a string or float64")
+		return errs.BadValue{
+			What: "umask", Valid: validUmaskMsg, Actual: vals.ToString(v)}
 	}
 
 	if umask < 0 || umask > 0o777 {
-		return errors.New("umask value outside the range [0..0o777]")
+		// TODO: Switch to `%O` when Go 1.15 is the minimum acceptable version.
+		// Until then the formatting of negative numbers will be weird.
+		return errs.OutOfRange{
+			What: "umask", ValidLow: 0, ValidHigh: 0o777, ValidFmt: "0o%o",
+			Actual: fmt.Sprintf("0o%o", umask)}
 	}
 
 	umaskMutex.Lock()

pkg/eval/unix/umask_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/elves/elvish/pkg/eval"
+	"github.com/elves/elvish/pkg/eval/errs"
 )
 
 var That = eval.That
@@ -18,7 +19,7 @@ func TestUmask(t *testing.T) {
 		// We have to start with a known umask value.
 		That(`unix:umask = 022`).Puts(),
 		That(`put $unix:umask`).Puts(`0o022`),
-		// Now verify that mutating the value and outputing it works.
+		// Verify that mutating the value and outputing the new value works.
 		That(`unix:umask = 23`).Puts(),
 		That(`put $unix:umask`).Puts(`0o023`),
 		That(`unix:umask = 0o75`).Puts(),
@@ -37,5 +38,23 @@ func TestUmask(t *testing.T) {
 		// We should be back to our expected umask given the preceding tests
 		// applied a temporary change to that process attribute.
 		That(`put $unix:umask`).Puts(`0o075`),
+		// An explicit float64 value should be handled correctly.
+		That(`unix:umask=(float64 0o17) put $unix:umask`).Puts(`0o017`),
+		That(`unix:umask=(float64 123.4)`).ThrowsCause(errs.BadValue{
+			What: "umask", Valid: validUmaskMsg, Actual: "123.4"}),
+		// An invalid string should raise the expected exception.
+		That(`unix:umask=022z`).ThrowsCause(errs.BadValue{
+			What: "umask", Valid: validUmaskMsg, Actual: "022z"}),
+		// An invalid data type should raise the expected exception.
+		That(`unix:umask=[1]`).ThrowsCause(errs.BadValue{
+			What: "umask", Valid: validUmaskMsg, Actual: "[1]"}),
+		// Values outside the legal range should raise the expected exception.
+		//
+		// TODO: Switch to `%O` when Go 1.15 is the minimum acceptable version.
+		// Until then the formatting of negative numbers will be weird.
+		That(`unix:umask=0o1000`).ThrowsCause(errs.OutOfRange{
+			What: "umask", ValidLow: 0, ValidHigh: 0o777, ValidFmt: "0o%o", Actual: "0o1000"}),
+		That(`unix:umask=-1`).ThrowsCause(errs.OutOfRange{
+			What: "umask", ValidLow: 0, ValidHigh: 0o777, ValidFmt: "0o%o", Actual: "0o-1"}),
 	)
 }