diff --git a/README.md b/README.md index 14fa74b..4a53503 100644 --- a/README.md +++ b/README.md @@ -160,7 +160,7 @@ A `-H` value needs a colon. A bare name exits `71` rather than being sent as a h | Flag | Description | |------|-------------| | `--assert-ok` | Assert the status is not an error (2xx or 3xx) | -| `--assert-status` | Assert specific status code | +| `--assert-status` | Assert the status: a code, a class like `2xx`, a range like `401-403`, or a list | | `--assert-header` | Assert any value of the header matches a regex; a name alone asserts presence | | `--assert-header-eq` | Assert any value of the header equals the given value; a name alone asserts presence | | `--assert-header-missing` | Assert header is not present | @@ -171,6 +171,21 @@ A `-H` value needs a colon. A bare name exits `71` rather than being sent as a h | `--assert-redirect` | Assert redirect location matches regex | | `--assert-redirect-eq` | Assert redirect location equals exact value | +`--assert-status` accepts more than one code. A class matches its hundred, a +range matches its span inclusively, and a comma-separated list matches any +entry — and they mix: + +```bash +http-assert --assert-status 2xx https://example.com # any 200-299 +http-assert --assert-status 200,204 https://example.com # either +http-assert --assert-status 401-403 https://example.com # any of the three +http-assert --assert-status 301,2xx https://example.com # mixed +``` + +A code no response can carry — `-1`, `1000`, `099` — is rejected before the +request is made, exiting `71`. A typo in the invocation is not a fact about the +service, so it must not arrive as `93`. + The three header flags and `--assert-jq` can be repeated to make several assertions of that kind. Every other assertion flag takes a single value; giving one twice exits `71` rather than silently keeping the last. A response header can carry several values, which is a different thing from repeating the flag. `Set-Cookie` routinely does, and `--assert-header` and `--assert-header-eq` hold when **any** value matches: diff --git a/assertions.go b/assertions.go index a48de61..34a4955 100644 --- a/assertions.go +++ b/assertions.go @@ -100,6 +100,128 @@ func headerValues(vs []string) string { return strings.Join(quoted, ", ") } +// Status codes a response can actually carry. net/http refuses to write +// anything outside this range -- 99 and 1000 panic in WriteHeader -- so a spec +// naming one of them is a typo in the invocation rather than a fact about the +// service, and it is rejected before a request is made. +// +// The upper bound is 999 rather than 599 because 6xx-9xx are non-conformant +// but observable: a server can send them and net/http reports them faithfully, +// so an assertion about one is answerable. +const ( + statusMin = 100 + statusMax = 999 +) + +// statusRange is an inclusive span of status codes. A single code is a span of +// one, so every form --assert-status accepts reduces to the same shape. +type statusRange struct{ lo, hi int } + +// statusSpec is the set of status codes an assertion will accept. +// +// text is kept as the caller wrote it: a failure saying `expected 2xx` is the +// question they asked, where `expected 200-299` would be an answer they would +// have to translate back. +type statusSpec struct { + text string + ranges []statusRange +} + +func (s statusSpec) matches(code int) bool { + for _, r := range s.ranges { + if code >= r.lo && code <= r.hi { + return true + } + } + + return false +} + +// parseStatusSpec reads the forms --assert-status accepts: an exact code, a +// class like 2xx, an inclusive range like 401-403, or a comma-separated list +// mixing any of them. +// +// Parsed by hand rather than by regexp, which the linter forbids compiling +// from user input anyway, and which would be longer than the three cases it +// replaced. +func parseStatusSpec(text string) (statusSpec, error) { + spec := statusSpec{text: text} + if strings.TrimSpace(text) == "" { + return spec, fmt.Errorf("it is empty; give a status code, a class like 2xx, or a range like 401-403") + } + + for _, term := range strings.Split(text, ",") { + r, err := parseStatusTerm(strings.TrimSpace(term)) + if err != nil { + return spec, err + } + spec.ranges = append(spec.ranges, r) + } + + return spec, nil +} + +func parseStatusTerm(term string) (statusRange, error) { + if term == "" { + return statusRange{}, fmt.Errorf("it has an empty entry; remove the stray comma") + } + + // A class: the leading digit fixes the hundred, xx covers the rest. + if len(term) == 3 && isX(term[1]) && isX(term[2]) { + d := term[0] + if d < '1' || d > '9' { + return statusRange{}, fmt.Errorf("%q is not a status class; the leading digit is 1 to 9", term) + } + lo := int(d-'0') * 100 + + return statusRange{lo, lo + 99}, nil + } + + // A range. Both ends must be present, so a negative number falls through + // to be reported as the code it is not, rather than as a range with an + // empty low end -- which named "" in the error and not what was typed. + if loText, hiText, isRange := strings.Cut(term, "-"); isRange && loText != "" && hiText != "" { + lo, err := parseStatusCode(loText) + if err != nil { + return statusRange{}, err + } + hi, err := parseStatusCode(hiText) + if err != nil { + return statusRange{}, err + } + if lo > hi { + return statusRange{}, fmt.Errorf("range %q counts down; write it low to high", term) + } + + return statusRange{lo, hi}, nil + } + + code, err := parseStatusCode(term) + + return statusRange{code, code}, err +} + +func isX(b byte) bool { return b == 'x' || b == 'X' } + +func parseStatusCode(text string) (int, error) { + if len(text) != 3 { + return 0, fmt.Errorf("%q is not a three-digit status code", text) + } + for i := 0; i < len(text); i++ { + if text[i] < '0' || text[i] > '9' { + return 0, fmt.Errorf("%q is not a three-digit status code", text) + } + } + + code, _ := strconv.Atoi(text) // three digits, so it cannot overflow + if code < statusMin || code > statusMax { + return 0, fmt.Errorf("no response can carry status %s; codes run %d to %d", + text, statusMin, statusMax) + } + + return code, nil +} + func AssertStatusOK() Assertion { return newAssertion("ok", func(res *httpResponse) (*Failure, error) { if s := res.StatusCode; s < 200 || s >= 400 { @@ -130,14 +252,15 @@ func AssertStatusNOK() Assertion { }) } -func AssertStatusEqual(expStatus int) Assertion { +// AssertStatus holds when the response carries any status the spec names. +func AssertStatus(spec statusSpec) Assertion { return newAssertion("status", func(res *httpResponse) (*Failure, error) { - if res.StatusCode != expStatus { + if !spec.matches(res.StatusCode) { return &Failure{ - Expected: expStatus, + Expected: spec.text, Actual: res.StatusCode, - Message: fmt.Sprintf("status: expected %d, got %d (%q)", - expStatus, res.StatusCode, res.Status), + Message: fmt.Sprintf("status: expected %s, got %d (%q)", + spec.text, res.StatusCode, res.Status), }, nil } diff --git a/assertions_test.go b/assertions_test.go index 5889e26..a887315 100644 --- a/assertions_test.go +++ b/assertions_test.go @@ -58,7 +58,20 @@ func Test_AssertStatusOK(t *testing.T) { } } -func Test_AssertStatusEqual(t *testing.T) { +// mustSpec parses a spec that the test author asserts is valid. Parsing rather +// than constructing keeps the tests honest about the only path a caller has. +func mustSpec(t *testing.T, text string) statusSpec { + t.Helper() + + spec, err := parseStatusSpec(text) + if err != nil { + t.Fatalf("parseStatusSpec(%q): unexpected error: %s", text, err) + } + + return spec +} + +func Test_AssertStatus(t *testing.T) { t.Parallel() testCases := []struct { @@ -81,9 +94,15 @@ func Test_AssertStatusEqual(t *testing.T) { {914, "Custom Response", false}, } - // 1 is never a real status, so that assertion must always fail; 200 and 429 - // appear in the table and must pass for their own case only. - assertions := map[int]Assertion{1: AssertStatusEqual(1), 200: AssertStatusEqual(200), 429: AssertStatusEqual(429)} + // 599 appears in no case above, so that assertion must always fail; 200 and + // 429 appear in the table and must pass for their own case only. It used to + // be 1, which is no longer expressible: a spec naming a code no response + // can carry is now rejected at the flag rather than failing at runtime. + assertions := map[int]Assertion{ + 599: AssertStatus(mustSpec(t, "599")), + 200: AssertStatus(mustSpec(t, "200")), + 429: AssertStatus(mustSpec(t, "429")), + } for _, tc := range testCases { t.Run(strconv.Itoa(tc.StatusCode), func(t *testing.T) { res := &httpResponse{ @@ -93,7 +112,7 @@ func Test_AssertStatusEqual(t *testing.T) { }, } - for _, expected := range []int{1, 200, 429} { + for _, expected := range []int{599, 200, 429} { want := "" if tc.StatusCode != expected { want = fmt.Sprintf("status: expected %d, got %d (%q)", expected, tc.StatusCode, tc.Status) @@ -645,9 +664,9 @@ func Test_AssertionIdentity(t *testing.T) { Kind: "nok", Expected: "not 2xx-3xx", Actual: 200, }, { - Name: "status", Assertion: AssertStatusEqual(200), + Name: "status", Assertion: AssertStatus(mustSpec(t, "200")), Res: statusRes(500, "500 Internal Server Error"), - Kind: "status", Expected: 200, Actual: 500, + Kind: "status", Expected: "200", Actual: 500, }, { Name: "header present", Assertion: AssertHeaderPresent("X-Absent"), diff --git a/e2e_status_test.go b/e2e_status_test.go new file mode 100644 index 0000000..ab6e908 --- /dev/null +++ b/e2e_status_test.go @@ -0,0 +1,79 @@ +package main_test + +import "testing" + +// TestE2EStatusSpec covers the forms --assert-status accepts end to end, and +// the rejection that keeps a typo out of the assertion exit code (#93). +func TestE2EStatusSpec(t *testing.T) { + // /ok answers 200, /created 201, /500 500. + t.Run("an exact code still works", func(t *testing.T) { + assertExit(t, run(t, nil, "--assert-status", "200", url("/ok")), exitOK) + assertExit(t, run(t, nil, "--assert-status", "200", url("/500")), exitAssertFail) + }) + + t.Run("a class matches its hundred", func(t *testing.T) { + assertExit(t, run(t, nil, "--assert-status", "2xx", url("/ok")), exitOK) + assertExit(t, run(t, nil, "--assert-status", "2xx", url("/created")), exitOK) + assertExit(t, run(t, nil, "--assert-status", "2xx", url("/500")), exitAssertFail) + assertExit(t, run(t, nil, "--assert-status", "5xx", url("/500")), exitOK) + }) + + t.Run("a list matches any of its entries", func(t *testing.T) { + assertExit(t, run(t, nil, "--assert-status", "200,201", url("/ok")), exitOK) + assertExit(t, run(t, nil, "--assert-status", "200,201", url("/created")), exitOK) + assertExit(t, run(t, nil, "--assert-status", "204,301", url("/ok")), exitAssertFail) + }) + + t.Run("a range matches its span, inclusive at both ends", func(t *testing.T) { + assertExit(t, run(t, nil, "--assert-status", "200-201", url("/ok")), exitOK) + assertExit(t, run(t, nil, "--assert-status", "200-201", url("/created")), exitOK) + assertExit(t, run(t, nil, "--assert-status", "201-300", url("/ok")), exitAssertFail) + }) + + t.Run("the forms mix in one spec", func(t *testing.T) { + assertExit(t, run(t, nil, "--assert-status", "301,2xx,500-503", url("/ok")), exitOK) + assertExit(t, run(t, nil, "--assert-status", "301,2xx,500-503", url("/500")), exitOK) + assertExit(t, run(t, nil, "--assert-status", "301,404,410-418", url("/ok")), exitAssertFail) + }) + + // The failure quotes the spec as written rather than an expansion of it. + t.Run("the failure names the spec the caller wrote", func(t *testing.T) { + r := run(t, nil, "--assert-status", "4xx", url("/ok")) + assertExit(t, r, exitAssertFail) + assertContains(t, r, `status: expected 4xx, got 200 ("200 OK")`) + + r = run(t, nil, "--assert-status", "301,410-418", url("/ok")) + assertContains(t, r, `status: expected 301,410-418, got 200 ("200 OK")`) + }) + + // A spec no response can satisfy is a typo, so it must not reach exit 93 -- + // a CI job reading that code would conclude the service was broken (#93). + t.Run("an unusable spec is rejected before the request", func(t *testing.T) { + for _, tc := range []struct{ Spec, Want string }{ + {"-1", "not a three-digit status code"}, + {"1000", "not a three-digit status code"}, + {"099", "no response can carry status"}, + {"0xx", "not a status class"}, + {"403-401", "counts down"}, + {"200,,204", "empty entry"}, + {"nonsense", "not a three-digit status code"}, + } { + r := run(t, nil, "--assert-status", tc.Spec, url("/ok")) + assertExit(t, r, exitBadInvocation) + assertContains(t, r, "Invalid value for --assert-status flag") + assertContains(t, r, tc.Want) + } + }) + + // Rejection happens at the flag, so no request is made at all. + t.Run("a rejected spec is still rejected against an unreachable host", func(t *testing.T) { + r := run(t, nil, "--assert-status", "1000", "http://127.0.0.1:9/never") + assertExit(t, r, exitBadInvocation) + }) + + t.Run("it is still single-valued", func(t *testing.T) { + r := run(t, nil, "--assert-status", "200", "--assert-status", "2xx", url("/ok")) + assertExit(t, r, exitBadInvocation) + assertContains(t, r, "accepts a single value") + }) +} diff --git a/flags_test.go b/flags_test.go index d4db523..ac9b989 100644 --- a/flags_test.go +++ b/flags_test.go @@ -80,7 +80,7 @@ func Test_rejectRepeats(t *testing.T) { t.Parallel() fs := pflag.NewFlagSet("test", pflag.ContinueOnError) - fs.Int("assert-status", 0, "") + fs.String("assert-status", "", "") fs.String("assert-body", "", "") fs.Bool("assert-ok", false, "") fs.StringArray("assert-header", nil, "") @@ -117,11 +117,11 @@ func Test_checkRepeats_allows(t *testing.T) { t.Parallel() fs := pflag.NewFlagSet("test", pflag.ContinueOnError) - fs.Int("assert-status", 0, "") + fs.String("assert-status", "", "") fs.StringArray("assert-header", nil, "") rejectRepeats(fs) - if err := fs.Parse([]string{"--assert-status", "1", "--assert-header", "a", "--assert-header", "b"}); err != nil { + if err := fs.Parse([]string{"--assert-status", "200", "--assert-header", "a", "--assert-header", "b"}); err != nil { t.Fatalf("parse: %s", err) } diff --git a/main.go b/main.go index 304f508..8d7c557 100644 --- a/main.go +++ b/main.go @@ -142,6 +142,18 @@ and that is a different thing from repeating the flag. --assert-header and cookies. --assert-header-missing is the strict one: it fails if the header carries any value at all. +--assert-status takes more than one code. A class matches its hundred, a range +matches its span inclusively, and a comma-separated list matches any entry; +they mix freely: + + --assert-status 2xx any 200-299 + --assert-status 200,204 either of those two + --assert-status 401-403 any of 401, 402, 403 + --assert-status 301,2xx a class and a code together + +A code no response can carry is rejected before the request is made, so a typo +exits 71 rather than reporting a service that answered perfectly well as wrong. + The two boolean assertions can be negated with =false, which selects the opposite assertion rather than cancelling the flag: --assert-ok=false asserts the status IS an error, and --assert-body-empty=false asserts the body is not @@ -789,7 +801,8 @@ func parseHostMappings(vals []string) ([]hostMapping, error) { } func registerAssertionFlags(cmd *cobra.Command) { - cmd.Flags().Int("assert-status", 0, "Assert response status equals the provided value") + cmd.Flags().String("assert-status", "", + "Assert response status; a code, a class like 2xx, a range like 401-403, or a list of those") cmd.Flags().StringArray("assert-header", nil, "Assert any value of the header matches the provided regexp; NAME alone asserts it is present") cmd.Flags().StringArray("assert-header-eq", nil, @@ -916,8 +929,12 @@ func parseAssertionFlags(cmd *cobra.Command) []Assertion { } if cmd.Flags().Changed("assert-status") { - s, _ := cmd.Flags().GetInt("assert-status") - res = append(res, AssertStatusEqual(s)) + v, _ := cmd.Flags().GetString("assert-status") + spec, err := parseStatusSpec(v) + if err != nil { + dief(exitBadInvocation, "Invalid value for --assert-status flag: %s", err) + } + res = append(res, AssertStatus(spec)) } if cmd.Flags().Changed("assert-header") { diff --git a/status_test.go b/status_test.go new file mode 100644 index 0000000..a849ef6 --- /dev/null +++ b/status_test.go @@ -0,0 +1,96 @@ +package main + +import ( + "strings" + "testing" +) + +// Test_parseStatusSpec covers what the flag accepts and what it turns away. +// +// Rejection matters as much as acceptance here: a spec naming a code no +// response can carry is a typo in the command line, and reporting it as a +// failed assertion would tell a CI job the service is broken (#93). +func Test_parseStatusSpec(t *testing.T) { + t.Parallel() + + t.Run("accepted", func(t *testing.T) { + tests := []struct { + Spec string + Matches []int + Misses []int + }{ + {"200", []int{200}, []int{199, 201, 500}}, + {"2xx", []int{200, 201, 250, 299}, []int{199, 300}}, + {"2XX", []int{200, 299}, []int{300}}, + {"200,204", []int{200, 204}, []int{201, 203, 205}}, + {"200, 204", []int{200, 204}, []int{202}}, + {"401-403", []int{401, 402, 403}, []int{400, 404}}, + {"200-200", []int{200}, []int{199, 201}}, + {"301,2xx,500-503", []int{200, 299, 301, 500, 503}, []int{300, 302, 499, 504}}, + // Allowed without being advertised: a class outside 1xx-5xx is + // non-conformant but observable, so an assertion about it is + // answerable rather than nonsense. + {"9xx", []int{900, 999}, []int{899}}, + {"1xx", []int{100, 199}, []int{200}}, + {"999", []int{999}, []int{998}}, + {"100", []int{100}, []int{101}}, + } + + for _, tc := range tests { + t.Run(tc.Spec, func(t *testing.T) { + spec, err := parseStatusSpec(tc.Spec) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + if spec.text != tc.Spec { + t.Errorf("text = %q, want %q; the failure quotes what was written", spec.text, tc.Spec) + } + for _, code := range tc.Matches { + if !spec.matches(code) { + t.Errorf("%q does not match %d, but should", tc.Spec, code) + } + } + for _, code := range tc.Misses { + if spec.matches(code) { + t.Errorf("%q matches %d, but should not", tc.Spec, code) + } + } + }) + } + }) + + t.Run("rejected", func(t *testing.T) { + tests := []struct{ Spec, Want string }{ + {"", "it is empty"}, + {" ", "it is empty"}, + {"-1", "not a three-digit status code"}, + {"1000", "not a three-digit status code"}, + {"99", "not a three-digit status code"}, + {"20", "not a three-digit status code"}, + {"abc", "not a three-digit status code"}, + {"2x", "not a three-digit status code"}, + {"099", "no response can carry status"}, + {"000", "no response can carry status"}, + {"0xx", "not a status class"}, + {"403-401", "counts down"}, + {"200,,204", "empty entry"}, + {"200,", "empty entry"}, + {"200-", "not a three-digit status code"}, + {"200,999x", "not a three-digit status code"}, + // One bad term poisons the whole spec rather than being skipped. + {"200,1000", "not a three-digit status code"}, + } + + for _, tc := range tests { + t.Run(tc.Spec, func(t *testing.T) { + _, err := parseStatusSpec(tc.Spec) + if err == nil { + t.Fatalf("expected %q to be rejected", tc.Spec) + } + if !strings.Contains(err.Error(), tc.Want) { + t.Errorf("error = %q, want it to mention %q", err, tc.Want) + } + }) + } + }) +}