Incorporate review comments

Signed-off-by: Prasad Chandrasekaran <[email protected]>

runtime/http.go

@@ -94,9 +94,13 @@ func (*httpHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 			w.Write([]byte(strings.Join(lines, "\n") + "\n"))
 		} else if strings.HasSuffix(key, "/count") {
 			fp := key[:len(key)-len("/count")]
-			_, count, err := Status(fp)
+			_, count, err := status(fp)
 			if err != nil {
-				http.Error(w, "failed to GET: "+err.Error(), http.StatusNotFound)
+				if err == ErrNoExist {
+					http.Error(w, "failed to GET: "+err.Error(), http.StatusNotFound)
+				} else {
+					http.Error(w, "failed to GET: "+err.Error(), http.StatusInternalServerError)
+				}
 				return
 			}
 			w.Write([]byte(strconv.Itoa(count)))

runtime/termscounter_test.go

@@ -1,123 +1,131 @@
 package runtime_test
 
 import (
-	"testing"
-	"time"
-
 	"go.etcd.io/gofail/runtime"
+	"testing"
 )
 
-var __fp_ExampleString *runtime.Failpoint = runtime.NewFailpoint("ExampleString") //nolint:stylecheck
+var __fp_ExampleString *runtime.Failpoint
+
+// check if failpoint is initialized as gofail
+// tests can clear global variables of runtime packages
+func initFP() {
+	if __fp_ExampleString == nil {
+		__fp_ExampleString = runtime.NewFailpoint("ExampleString") //nolint:stylecheck
+	}
+}
 
 func TestTermsCounter(t *testing.T) {
 	testcases := []struct {
-		name      string
-		fp        string
-		desc      string
-		runbefore int
-		runafter  int
-		want      int
+		name              string
+		fp                string
+		failpointTerm     string
+		runBeforeEnabling int
+		runAfterEnabling  int
+		wantCount         int
 	}{
 		{
-			name:     "Terms limit Failpoint",
-			fp:       "ExampleString",
-			desc:     `10*sleep(10)->1*return("abc")`,
-			runafter: 12,
-			want:     11,
+			name:             "Terms limit Failpoint",
+			fp:               "ExampleString",
+			failpointTerm:    `10*sleep(10)->1*return("abc")`,
+			runAfterEnabling: 12,
+			// This refers to the number of term executions and is
+			// independent of caller execution frequency. E.g., Run
+			// failpoint actions only 11 times even if we have a greater
+			// number of callsite executions (12 in this case)
+			wantCount: 11,
 		},
 		{
-			name:      "Inbetween Enabling Failpoint",
-			fp:        "ExampleString",
-			desc:      `10*sleep(10)->1*return("abc")`,
-			runbefore: 2,
-			runafter:  3,
-			want:      3,
+			name:              "Inbetween Enabling Failpoint",
+			fp:                "ExampleString",
+			failpointTerm:     `10*sleep(10)->1*return("abc")`,
+			runBeforeEnabling: 2,
+			runAfterEnabling:  3,
+			wantCount:         3,
 		},
 		{
-			name:      "Before Enabling Failpoint",
-			fp:        "ExampleString",
-			desc:      `10*sleep(10)->1*return("abc")`,
-			runbefore: 2,
-			runafter:  0,
-			want:      0,
+			name:              "Before Enabling Failpoint",
+			fp:                "ExampleString",
+			failpointTerm:     `10*sleep(10)->1*return("abc")`,
+			runBeforeEnabling: 2,
+			runAfterEnabling:  0,
+			wantCount:         0,
 		},
 	}
 
+	initFP()
 	for _, tc := range testcases {
 		t.Run(tc.name, func(t *testing.T) {
 
-			for i := 0; i < tc.runbefore; i++ {
+			for i := 0; i < tc.runBeforeEnabling; i++ {
 				exampleFunc()
-				time.Sleep(10 * time.Millisecond)
 			}
 
-			err := runtime.Enable(tc.fp, tc.desc)
+			err := runtime.Enable(tc.fp, tc.failpointTerm)
 			if err != nil {
 				t.Fatal(err)
 			}
 			defer runtime.Disable(tc.fp)
-			for i := 0; i < tc.runafter; i++ {
+			for i := 0; i < tc.runAfterEnabling; i++ {
 				exampleFunc()
-				time.Sleep(10 * time.Millisecond)
 			}
 			_, count, err := runtime.Status(tc.fp)
 			if err != nil {
 				t.Fatal(err)
 			}
-			if tc.want != count {
+			if tc.wantCount != count {
 				t.Fatal("counter is not properly incremented")
 			}
 		})
 	}
 }
 
-func TestResetingCounterOnTerm(t *testing.T) {
+func TestEnablingNewTermResetsCount(t *testing.T) {
 	testcases := []struct {
-		name      string
-		fp        string
-		desc      string
-		newdesc   string
-		runbefore int
-		runafter  int
-		want      int
+		name       string
+		fp         string
+		oldTerm    string
+		newTerm    string
+		runOldTerm int
+		runNewTerm int
+		wantCount  int
 	}{
 		{
-			name:      "Change and Reset Counter",
-			fp:        "ExampleString",
-			desc:      `10*sleep(10)->1*return("abc")`,
-			newdesc:   "sleep(10)",
-			runbefore: 2,
-			runafter:  3,
-			want:      3,
+			name:       "Change and Reset Counter",
+			fp:         "ExampleString",
+			oldTerm:    `10*sleep(10)->1*return("abc")`,
+			newTerm:    "sleep(10)",
+			runOldTerm: 2,
+			runNewTerm: 3,
+			wantCount:  3,
 		},
 	}
 
+	initFP()
 	for _, tc := range testcases {
 		t.Run(tc.name, func(t *testing.T) {
-			err := runtime.Enable(tc.fp, tc.desc)
+			err := runtime.Enable(tc.fp, tc.oldTerm)
 			if err != nil {
 				t.Fatal(err)
 			}
 
-			for i := 0; i < tc.runbefore; i++ {
+			for i := 0; i < tc.runOldTerm; i++ {
 				exampleFunc()
-				time.Sleep(10 * time.Millisecond)
 			}
-			err = runtime.Enable(tc.fp, tc.newdesc)
+			err = runtime.Enable(tc.fp, tc.newTerm)
 			if err != nil {
 				t.Fatal(err)
 			}
 			defer runtime.Disable(tc.fp)
 
-			for i := 0; i < tc.runafter; i++ {
+			for i := 0; i < tc.runNewTerm; i++ {
 				exampleFunc()
-				time.Sleep(10 * time.Millisecond)
 			}
 			_, count, err := runtime.Status(tc.fp)
 			if err != nil {
 				t.Fatal(err)
 			}
-			if tc.want != count {
+			if tc.wantCount != count {
 				t.Fatal("counter is not properly incremented")
 			}
 		})