Skip to content

[tests] extism Eval() error paths need expanded mocking for plugin/instance failures #109

@robbyt

Description

@robbyt

Summary

engines/extism/evaluator/evaluator.go:133-134 carries a stale TODO:

// Eval implements evaluation.Evaluator
// TODO: Some error paths in this method are hard to test with the current design
// Consider adding more integration tests to cover these paths.

The TODO is being removed under #99, but the underlying gap deserves a real issue. Coverage for the package is ~93%; the remaining branches in Eval() and the helpers it calls (exec, execHelper) aren't easily reachable today because the path to a real failing WASM instance runs through a concrete *extismSDK.CompiledPlugin.

Branches needing better coverage

Eval() (already largely testable)

Most early guards are tested:

  • execUnit == nil (L137-138) ✓
  • GetContent() == nil (L141-143) ✓
  • GetByteCode() == nil (L146-149) ✓
  • exeID == "" (L152-155) — testable via custom executable unit
  • type assertion failure (L159-165) — testable with a non-*compiler.Executable content
  • GetExtismByteCode() == nil (L166-169) — testable
  • loadInputData failure (L172-175) — testable with a mock data provider
  • ConvertToExtismFormat failure (L178-181) — testable with adversarial input (e.g. unmarshalable types)
  • exec(...) failure (L184-191) — partially testable via the mocked plugin pattern below

exec() and execHelper() (the harder gaps)

engines/extism/evaluator/evaluator.go:105-130 and :62-101:

File:line Branch Why it's hard today
evaluator.go:114-117 plugin.Instance(ctx, ...) failing plugin is adapters.CompiledPlugin already, but tests don't yet drop in a fake whose Instance() returns an error.
evaluator.go:118-122 instance.Close(ctx) failing Logged-only path; coverage value low.
evaluator.go:79-82 exit != 0 This branch's "should we return output?" question is already tracked under #94 and is out of scope here.
evaluator.go:73-78 instance.CallWithContext returning a non-cancellation error Partially covered via mocked plugin instance; the cancellation-vs-other distinction (if ctx.Err() != nil) deserves explicit cases.
evaluator.go:88-91 json.Decoder.Decode failing → fallback to raw string Trivially exercisable with non-JSON output bytes; just missing a test.

Proposal

The adapters.CompiledPlugin and adapters.SdkPluginInstanceConfig interfaces already exist in engines/extism/adapters/interfaces.go, so the seam is mostly there. Two missing pieces:

  1. A reusable mock CompiledPlugin (and corresponding Instance) in the evaluator test suite, scriptable for: Instance returning an error, Instance returning a plugin whose CallWithContext errors with various shapes, Close returning an error.
  2. Tests that wire a *script.ExecutableUnit whose Content is a *compiler.Executable populated with the mock plugin, then call Eval(ctx) end-to-end with each scripted failure.

Concrete subtests to add:

  • TestEval_PluginInstanceError
  • TestEval_CallWithContextError (separate cancelled-vs-not subcase)
  • TestEval_NonJSONOutputFallsBackToString
  • TestEval_ConvertToExtismFormatError (adversarial input)
  • TestEval_LoadInputDataError

Out of scope

Files

  • engines/extism/evaluator/evaluator.go — function under test
  • engines/extism/evaluator/evaluator_test.go — new subtests
  • engines/extism/adapters/interfaces.go — confirm interface surface is sufficient
  • New: engines/extism/evaluator/mocks_test.go (or extend existing scaffolding)

This was previously tracked inline as a TODO at evaluator.go:133-134. Filed in response to issue #99's request to either resolve TODOs or convert them to issues.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions