Skip to content

fix(matchers): an empty map matcher panics instead of reporting a syntax error - #1118

Open
VXNCXNX wants to merge 1 commit into
goss-org:masterfrom
VXNCXNX:fix/empty-map-matcher-panic
Open

fix(matchers): an empty map matcher panics instead of reporting a syntax error#1118
VXNCXNX wants to merge 1 commit into
goss-org:masterfrom
VXNCXNX:fix/empty-map-matcher-panic

Conversation

@VXNCXNX

@VXNCXNX VXNCXNX commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

An empty map used as a matcher crashes goss instead of reporting a syntax error.

command:
  echo hi:
    exit-status: 0
    stdout: {}
before:
panic: runtime error: index out of range [0] with length 0
	.../resource/gomega.go:45
exit=2

after:
Command: echo hi: stdout:
Error
    Syntax Error: Invalid matcher configuration. An empty map asserts nothing, exactly one matcher is required
Count: 2, Failed: 1, Skipped: 0
exit=1

Same for listening: {} and for a nested one such as {"and": [{}]}.

Cause

matcherToGomegaMatcher guards the "too many keys" case but not the "no keys"
case, then indexes unconditionally:

keys := lo.Keys(matcherMap)
if len(keys) > 1 {
    return nil, fmt.Errorf("Syntax Error: ... Found multiple matchers: %q", keys)
}
key := keys[0]

An empty map reaches keys[0] with a zero-length slice.

The fix

The mirror image of the existing guard: return a syntax error when there are no
keys, in the same style and phrasing as the neighbouring one, so a malformed
gossfile is reported as malformed rather than crashing the run.

Verification

TestMatcherToGomegaMatcherEmptyMap covers the empty map at the top level and
nested inside and and not.

Two mutations, because "it panics" and "it returns the right thing" are separate
claims:

Removing the guard reproduces the original crash:

--- FAIL: TestMatcherToGomegaMatcherEmptyMap/top_level
panic: runtime error: index out of range [0] with length 0 [recovered, repanicked]

Returning a matcher instead of the error fails on the assertion, not on a panic,
which is what shows the test checks the behaviour rather than merely surviving:

Error: Expected nil, but got: &matchers.EqualMatcher{...Expected:interface {}(nil)}
Error: Expected error with "Syntax Error: Invalid matcher configuration. An empty map asserts nothing, exactly one matcher is required" in chain but got nil.

go test ./... is ok across all packages, and golangci-lint run ./resource/...
reports 0 issues.

On the golden fixtures: this change only affects an input that previously
crashed, so no generated output string changes. I grepped integration-tests/
and docs/ for both the old and new strings and there are no hits, so no
goss-expected*.yaml needs updating.

Checklist
  • make test-all (UNIX) passes. CI will also test this (could not run in full here: it needs Docker. go test ./... and golangci-lint pass, and the before/after above comes from running real binaries built from each tree)
  • unit and/or integration tests are included (if applicable)
  • documentation is changed or added (if applicable) (no doc change: an empty map was never valid, it just failed badly)

One note unrelated to the fix: gofumpt -l resource/ lists gomega.go both
before and after this change, so I left the pre-existing formatting alone rather
than mixing a reformat into this diff.

Disclosure: written with AI assistance (Claude Code). I built binaries from master and from the patched tree, produced the before and after above by running them against the gossfile shown, and ran both mutation checks myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant