From a5fc0ef49d8822728167f353bff5adf9355ecf36 Mon Sep 17 00:00:00 2001 From: Prasad Chandrasekaran Date: Mon, 29 May 2023 10:31:40 +0530 Subject: [PATCH] Incorporate review comments Signed-off-by: Prasad Chandrasekaran --- runtime/http.go | 8 ++- runtime/termscounter_test.go | 118 +++++++++++++++++++---------------- 2 files changed, 69 insertions(+), 57 deletions(-) diff --git a/runtime/http.go b/runtime/http.go index 56bf2b1..74c4588 100644 --- a/runtime/http.go +++ b/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))) diff --git a/runtime/termscounter_test.go b/runtime/termscounter_test.go index 464b8f9..e803200 100644 --- a/runtime/termscounter_test.go +++ b/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") } })