Skip to content

fix(test): match xrayErrorCause by reflected type, not type name - #128

Closed
carole-lavillonniere wants to merge 1 commit into
localstackfrom
fix/jsonv2-rawmessage-mock-matcher
Closed

fix(test): match xrayErrorCause by reflected type, not type name#128
carole-lavillonniere wants to merge 1 commit into
localstackfrom
fix/jsonv2-rawmessage-mock-matcher

Conversation

@carole-lavillonniere

Copy link
Copy Markdown

Problem

localstack has been red since #122 (go directive → 1.27.0, merged Aug 22). Every build on the branch since then has failed, and six Renovate PRs automerged on top of the breakage.

Go 1.27 makes encoding/json's RawMessage an alias for jsontext.Value, so the runtime type name is now jsontext.Value. testify's AnythingOfType matches on that name string, so the three SendInvokeFinishedEvent expectations stopped matching and the mock panicked on an unexpected call:

mock: Unexpected Method Call
SendInvokeFinishedEvent(*interop.TracingCtx,jsontext.Value)
        1: jsontext.Value(nil)
The closest call I have is:
SendInvokeFinishedEvent(mock.anythingOfTypeArgument,mock.anythingOfTypeArgument)
        1: "json.RawMessage"
Diff: 1: FAIL:  type json.RawMessage != type Value - (jsontext.Value=null)

TestRuntimeErrorFailure_ErrorWhileError panics, which fails the package and the test job.

Fix

Switch the three expectations to mock.IsType(json.RawMessage(nil)). IsType resolves the type through reflect.TypeOf rather than comparing a hardcoded name, so it matches whether or not the alias is in play — no re-break if the alias changes again.

Hardcoding "jsontext.Value" would also go green today, but re-breaks on any future rename and reads as if the test cares about the JSON encoder's internals; it doesn't.

Scope

Only AnythingOfType("json.RawMessage") was affected. I checked every AnythingOfType in the repo — the rest are int, bool, int64, time.Time, time.Duration, *interop.TracingCtx, interop.TelemetrySubscriptionMetrics, none of which are aliased.

Production code is untouched; this is a test-only change.

Verification

go test ./... passes locally on go1.27.0. Before the change, internal/lambda-managed-instances/invoke panicked.

Note

This fixes the breakage but not the reason it reached the branch. The ruleset requires the build check, but build has needs: test — so a failing test makes build report skipped, which GitHub counts as passing. test itself isn't required, so Renovate automerges onto red. Follow-up PR addresses that.

Go 1.27 makes encoding/json's RawMessage an alias for jsontext.Value, so
the runtime type name is now "jsontext.Value". testify's AnythingOfType
matches on that name string, so the three SendInvokeFinishedEvent
expectations stopped matching and the mock panicked on an unexpected call:

    Diff: 1: FAIL: type json.RawMessage != type Value - (jsontext.Value=null)

Switch to mock.IsType, which resolves the type through reflection and so
matches whether or not the alias is in play. This has broken
TestRuntimeErrorFailure_ErrorWhileError and left the localstack branch red
since the go directive bump to 1.27.0 in #122.
@carole-lavillonniere

Copy link
Copy Markdown
Author

Closing — wrong fix, and the framing in the description was wrong.

Go did not break backward compatibility here. jsonv2 is a GOEXPERIMENT gated by //go:build goexperiment.jsonv2, and the go.mod language directive is what enables it:

go.mod directive reflect.TypeOf(json.RawMessage(nil))
go 1.27.0 jsontext.Value
go 1.26.0 json.RawMessage
go 1.27.0 + GOEXPERIMENT=nojsonv2 json.RawMessage

So #122 (go 1.26.6go 1.27.0, automerged) opted the whole emulator into a different JSON implementation. The failing test is the canary for that, not the problem — and this PR would have silenced it while keeping the behavior change.

It also edits upstream code: internal/lambda-managed-instances/invoke/running_invoke_test.go exists verbatim at aws/aws-lambda-runtime-interface-emulator (upstream PR aws#158), so this would conflict at the next develop→localstack sync.

The unmodified upstream test passes with the directive back at 1.26.6. Replacement PR reverts the directive instead.

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