Refactors spectest harness (#1489)

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>

internal/integration_test/filecache/filecache_test.go

@@ -11,10 +11,9 @@ import (
 	"time"
 
 	"github.com/tetratelabs/wazero"
+	"github.com/tetratelabs/wazero/api"
 	"github.com/tetratelabs/wazero/experimental"
 	"github.com/tetratelabs/wazero/experimental/logging"
-	"github.com/tetratelabs/wazero/internal/engine/compiler"
-	"github.com/tetratelabs/wazero/internal/filecache"
 	"github.com/tetratelabs/wazero/internal/integration_test/spectest"
 	v1 "github.com/tetratelabs/wazero/internal/integration_test/spectest/v1"
 	"github.com/tetratelabs/wazero/internal/platform"
@@ -66,8 +65,10 @@ func TestSpecTestCompilerCache(t *testing.T) {
 		require.True(t, len(files) > 0)
 	} else {
 		// Run the spectest with the file cache.
-		fc := filecache.New(cacheDir)
+		cc, err := wazero.NewCompilationCacheWithDir(cacheDir)
-		spectest.Run(t, v1.Testcases, context.Background(), fc, compiler.NewEngine, v1.EnabledFeatures)
+		require.NoError(t, err)
+		spectest.Run(t, v1.Testcases, context.Background(),
+			wazero.NewRuntimeConfigCompiler().WithCompilationCache(cc).WithCoreFeatures(api.CoreFeaturesV1))
 	}
 }
 

internal/integration_test/spectest/spectest.go

@@ -10,17 +10,14 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/tetratelabs/wazero"
 	"github.com/tetratelabs/wazero/api"
-	"github.com/tetratelabs/wazero/internal/filecache"
 	"github.com/tetratelabs/wazero/internal/moremath"
-	"github.com/tetratelabs/wazero/internal/sys"
 	"github.com/tetratelabs/wazero/internal/testing/require"
 	"github.com/tetratelabs/wazero/internal/wasm"
-	binaryformat "github.com/tetratelabs/wazero/internal/wasm/binary"
 	"github.com/tetratelabs/wazero/internal/wasmruntime"
 )
 
-// TODO: complete porting this to wazero API
 type (
 	testbase struct {
 		SourceFile string    `json:"source_filename"`
@@ -316,43 +313,16 @@ func (c command) expectedError() (err error) {
 //
 //	cd testdata; wat2wasm --debug-names spectest.wat
 //
+// This module is required by some test cases, and instantiated before running cases.
+// See https://github.com/WebAssembly/spec/blob/wg-1.0/test/core/imports.wast
+// See https://github.com/WebAssembly/spec/blob/wg-1.0/interpreter/script/js.ml#L13-L25
+//
 //go:embed testdata/spectest.wasm
 var spectestWasm []byte
 
-// addSpectestModule adds a module that drops inputs and returns globals as 666 per the default test harness.
-//
-// See https://github.com/WebAssembly/spec/blob/wg-1.0/test/core/imports.wast
-// See https://github.com/WebAssembly/spec/blob/wg-1.0/interpreter/script/js.ml#L13-L25
-func addSpectestModule(t *testing.T, ctx context.Context, s *wasm.Store, enabledFeatures api.CoreFeatures) {
-	mod, err := binaryformat.DecodeModule(spectestWasm, api.CoreFeaturesV2, wasm.MemoryLimitPages, false, false, false)
-	require.NoError(t, err)
-
-	maybeSetMemoryCap(mod)
-	mod.BuildMemoryDefinitions()
-
-	err = mod.Validate(enabledFeatures)
-	require.NoError(t, err)
-
-	err = s.Engine.CompileModule(ctx, mod, nil, false)
-	require.NoError(t, err)
-
-	typeIDs, err := s.GetFunctionTypeIDs(mod.TypeSection)
-	require.NoError(t, err)
-
-	_, err = s.Instantiate(ctx, mod, mod.NameSection.ModuleName, sys.DefaultContext(nil), typeIDs)
-	require.NoError(t, err)
-}
-
-// maybeSetMemoryCap assigns wasm.Memory Cap to Min, which is what wazero.CompileModule would do.
-func maybeSetMemoryCap(mod *wasm.Module) {
-	if mem := mod.MemorySection; mem != nil {
-		mem.Cap = mem.Min
-	}
-}
-
 // Run runs all the test inside the testDataFS file system where all the cases are described
 // via JSON files created from wast2json.
-func Run(t *testing.T, testDataFS embed.FS, ctx context.Context, fc filecache.Cache, newEngine func(context.Context, api.CoreFeatures, filecache.Cache) wasm.Engine, enabledFeatures api.CoreFeatures) {
+func Run(t *testing.T, testDataFS embed.FS, ctx context.Context, config wazero.RuntimeConfig) {
 	files, err := testDataFS.ReadDir("testdata")
 	require.NoError(t, err)
 
@@ -378,50 +348,40 @@ func Run(t *testing.T, testDataFS embed.FS, ctx context.Context, fc filecache.Ca
 		wastName := basename(base.SourceFile)
 
 		t.Run(wastName, func(t *testing.T) {
-			s := wasm.NewStore(enabledFeatures, newEngine(ctx, enabledFeatures, fc))
+			r := wazero.NewRuntimeWithConfig(ctx, config)
-			addSpectestModule(t, ctx, s, enabledFeatures)
+			defer func() {
+				require.NoError(t, r.Close(ctx))
+			}()
 
-			var lastInstantiatedModuleName string
+			_, err := r.InstantiateWithConfig(ctx, spectestWasm, wazero.NewModuleConfig())
-			for _, c := range base.Commands {
+			require.NoError(t, err)
+
+			modules := make(map[string]api.Module)
+			var lastInstantiatedModule api.Module
+			for i := 0; i < len(base.Commands); i++ {
+				c := &base.Commands[i]
 				t.Run(fmt.Sprintf("%s/line:%d", c.CommandType, c.Line), func(t *testing.T) {
 					msg := fmt.Sprintf("%s:%d %s", wastName, c.Line, c.CommandType)
 					switch c.CommandType {
 					case "module":
 						buf, err := testDataFS.ReadFile(testdataPath(c.Filename))
 						require.NoError(t, err, msg)
-						mod, err := binaryformat.DecodeModule(buf, enabledFeatures, wasm.MemoryLimitPages, false, false, false)
-						require.NoError(t, err, msg)
-						require.NoError(t, mod.Validate(enabledFeatures))
-						mod.AssignModuleID(buf, false, false)
 
-						moduleName := c.Name
+						var registeredName string
-						if moduleName == "" {
+						if next := i + 1; next < len(base.Commands) && base.Commands[next].CommandType == "register" {
-							// Use the file name as the name.
+							registeredName = base.Commands[next].As
-							moduleName = c.Filename
+							i++ // Skip the entire "register" command.
 						}
-
+						mod, err := r.InstantiateWithConfig(ctx, buf, wazero.NewModuleConfig().WithName(registeredName))
-						maybeSetMemoryCap(mod)
-						mod.BuildMemoryDefinitions()
-						err = s.Engine.CompileModule(ctx, mod, nil, false)
 						require.NoError(t, err, msg)
-
+						if c.Name != "" {
-						typeIDs, err := s.GetFunctionTypeIDs(mod.TypeSection)
+							modules[c.Name] = mod
-						require.NoError(t, err)
-
-						_, err = s.Instantiate(ctx, mod, moduleName, nil, typeIDs)
-						lastInstantiatedModuleName = moduleName
-						require.NoError(t, err)
-					case "register":
-						src := c.Name
-						if src == "" {
-							src = lastInstantiatedModuleName
 						}
-						require.NoError(t, s.AliasModule(src, c.As))
+						lastInstantiatedModule = mod
-						lastInstantiatedModuleName = c.As
 					case "assert_return", "action":
-						moduleName := lastInstantiatedModuleName
+						m := lastInstantiatedModule
 						if c.Action.Module != "" {
-							moduleName = c.Action.Module
+							m = modules[c.Action.Module]
 						}
 						switch c.Action.ActionType {
 						case "invoke":
@@ -430,16 +390,17 @@ func Run(t *testing.T, testDataFS embed.FS, ctx context.Context, fc filecache.Ca
 							if c.Action.Module != "" {
 								msg += " in module " + c.Action.Module
 							}
-							vals, types, err := callFunction(s, ctx, moduleName, c.Action.Field, args...)
+							fn := m.ExportedFunction(c.Action.Field)
+							results, err := fn.Call(ctx, args...)
 							require.NoError(t, err, msg)
-							require.Equal(t, len(exps), len(vals), msg)
+							require.Equal(t, len(exps), len(results), msg)
 							laneTypes := map[int]string{}
 							for i, expV := range c.Exps {
 								if expV.ValType == "v128" {
 									laneTypes[i] = expV.LaneType
 								}
 							}
-							matched, valuesMsg := valuesEq(vals, exps, types, laneTypes)
+							matched, valuesMsg := valuesEq(results, exps, fn.Definition().ResultTypes(), laneTypes)
 							require.True(t, matched, msg+"\n"+valuesMsg)
 						case "get":
 							_, exps := c.getAssertReturnArgsExps()
@@ -448,22 +409,8 @@ func Run(t *testing.T, testDataFS embed.FS, ctx context.Context, fc filecache.Ca
 							if c.Action.Module != "" {
 								msg += " in module " + c.Action.Module
 							}
-							module := s.Module(moduleName)
+							global := m.ExportedGlobal(c.Action.Field)
-							require.NotNil(t, module)
-							global := module.ExportedGlobal(c.Action.Field)
 							require.NotNil(t, global)
-							var expType wasm.ValueType
-							switch c.Exps[0].ValType {
-							case "i32":
-								expType = wasm.ValueTypeI32
-							case "i64":
-								expType = wasm.ValueTypeI64
-							case "f32":
-								expType = wasm.ValueTypeF32
-							case "f64":
-								expType = wasm.ValueTypeF64
-							}
-							require.Equal(t, expType, global.Type(), msg)
 							require.Equal(t, exps[0], global.Get(), msg)
 						default:
 							t.Fatalf("unsupported action type type: %v", c)
@@ -473,12 +420,13 @@ func Run(t *testing.T, testDataFS embed.FS, ctx context.Context, fc filecache.Ca
 							// We don't support direct loading of wast yet.
 							buf, err := testDataFS.ReadFile(testdataPath(c.Filename))
 							require.NoError(t, err, msg)
-							requireInstantiationError(t, ctx, s, buf, msg)
+							_, err = r.InstantiateWithConfig(ctx, buf, wazero.NewModuleConfig())
+							require.Error(t, err, msg)
 						}
 					case "assert_trap":
-						moduleName := lastInstantiatedModuleName
+						m := lastInstantiatedModule
 						if c.Action.Module != "" {
-							moduleName = c.Action.Module
+							m = modules[c.Action.Module]
 						}
 						switch c.Action.ActionType {
 						case "invoke":
@@ -487,7 +435,7 @@ func Run(t *testing.T, testDataFS embed.FS, ctx context.Context, fc filecache.Ca
 							if c.Action.Module != "" {
 								msg += " in module " + c.Action.Module
 							}
-							_, _, err := callFunction(s, ctx, moduleName, c.Action.Field, args...)
+							_, err := m.ExportedFunction(c.Action.Field).Call(ctx, args...)
 							require.ErrorIs(t, err, c.expectedError(), msg)
 						default:
 							t.Fatalf("unsupported action type type: %v", c)
@@ -499,9 +447,9 @@ func Run(t *testing.T, testDataFS embed.FS, ctx context.Context, fc filecache.Ca
 						}
 						buf, err := testDataFS.ReadFile(testdataPath(c.Filename))
 						require.NoError(t, err, msg)
-						requireInstantiationError(t, ctx, s, buf, msg)
+						_, err = r.InstantiateWithConfig(ctx, buf, wazero.NewModuleConfig())
+						require.Error(t, err, msg)
 					case "assert_exhaustion":
-						moduleName := lastInstantiatedModuleName
 						switch c.Action.ActionType {
 						case "invoke":
 							args := c.getAssertReturnArgs()
@@ -509,7 +457,7 @@ func Run(t *testing.T, testDataFS embed.FS, ctx context.Context, fc filecache.Ca
 							if c.Action.Module != "" {
 								msg += " in module " + c.Action.Module
 							}
-							_, _, err := callFunction(s, ctx, moduleName, c.Action.Field, args...)
+							_, err := lastInstantiatedModule.ExportedFunction(c.Action.Field).Call(ctx, args...)
 							require.ErrorIs(t, err, wasmruntime.ErrRuntimeStackOverflow, msg)
 						default:
 							t.Fatalf("unsupported action type type: %v", c)
@@ -521,10 +469,12 @@ func Run(t *testing.T, testDataFS embed.FS, ctx context.Context, fc filecache.Ca
 						}
 						buf, err := testDataFS.ReadFile(testdataPath(c.Filename))
 						require.NoError(t, err, msg)
-						requireInstantiationError(t, ctx, s, buf, msg)
+						_, err = r.InstantiateWithConfig(ctx, buf, wazero.NewModuleConfig())
+						require.Error(t, err, msg)
 					case "assert_uninstantiable":
 						buf, err := testDataFS.ReadFile(testdataPath(c.Filename))
 						require.NoError(t, err, msg)
+						_, err = r.InstantiateWithConfig(ctx, buf, wazero.NewModuleConfig())
 						if c.Text == "out of bounds table access" {
 							// This is not actually an instantiation error, but assert_trap in the original wast, but wast2json translates it to assert_uninstantiable.
 							// Anyway, this spectest case expects the error due to active element offset ouf of bounds
@@ -533,27 +483,10 @@ func Run(t *testing.T, testDataFS embed.FS, ctx context.Context, fc filecache.Ca
 							//
 							// In practice, such a module instance can be used for invoking functions without any issue. In addition, we have to
 							// retain functions after the expected "instantiation" failure, so in wazero we choose to not raise error in that case.
-							mod, err := binaryformat.DecodeModule(buf, s.EnabledFeatures, wasm.MemoryLimitPages, false, false, false)
-							require.NoError(t, err, msg)
-
-							err = mod.Validate(s.EnabledFeatures)
-							require.NoError(t, err, msg)
-
-							mod.AssignModuleID(buf, false, false)
-
-							maybeSetMemoryCap(mod)
-							err = s.Engine.CompileModule(ctx, mod, nil, false)
-							require.NoError(t, err, msg)
-
-							typeIDs, err := s.GetFunctionTypeIDs(mod.TypeSection)
-							require.NoError(t, err)
-
-							_, err = s.Instantiate(ctx, mod, t.Name(), nil, typeIDs)
 							require.NoError(t, err, msg)
 						} else {
-							requireInstantiationError(t, ctx, s, buf, msg)
+							require.Error(t, err, msg)
 						}
-
 					default:
 						t.Fatalf("unsupported command type: %s", c)
 					}
@@ -563,33 +496,6 @@ func Run(t *testing.T, testDataFS embed.FS, ctx context.Context, fc filecache.Ca
 	}
 }
 
-func requireInstantiationError(t *testing.T, ctx context.Context, s *wasm.Store, buf []byte, msg string) {
-	mod, err := binaryformat.DecodeModule(buf, s.EnabledFeatures, wasm.MemoryLimitPages, false, false, false)
-	if err != nil {
-		return
-	}
-
-	err = mod.Validate(s.EnabledFeatures)
-	if err != nil {
-		return
-	}
-
-	mod.AssignModuleID(buf, false, false)
-
-	maybeSetMemoryCap(mod)
-	mod.BuildMemoryDefinitions()
-	err = s.Engine.CompileModule(ctx, mod, nil, false)
-	if err != nil {
-		return
-	}
-
-	typeIDs, err := s.GetFunctionTypeIDs(mod.TypeSection)
-	require.NoError(t, err)
-
-	_, err = s.Instantiate(ctx, mod, t.Name(), nil, typeIDs)
-	require.Error(t, err, msg)
-}
-
 // basename avoids filepath.Base to ensure a forward slash is used even in Windows.
 // See https://pkg.go.dev/embed#hdr-Directives
 func basename(path string) string {
@@ -771,11 +677,3 @@ func f64Equal(expected, actual float64) (matched bool) {
 	}
 	return
 }
-
-// callFunction is inlined here as the spectest needs to validate the signature was correct
-// TODO: This is likely already covered with unit tests!
-func callFunction(s *wasm.Store, ctx context.Context, moduleName, funcName string, params ...uint64) ([]uint64, []wasm.ValueType, error) {
-	fn := s.Module(moduleName).ExportedFunction(funcName)
-	results, err := fn.Call(ctx, params...)
-	return results, fn.Definition().ResultTypes(), err
-}

internal/integration_test/spectest/v1/spec_test.go

@@ -4,8 +4,8 @@ import (
 	"context"
 	"testing"
 
-	"github.com/tetratelabs/wazero/internal/engine/compiler"
+	"github.com/tetratelabs/wazero"
-	"github.com/tetratelabs/wazero/internal/engine/interpreter"
+	"github.com/tetratelabs/wazero/api"
 	"github.com/tetratelabs/wazero/internal/integration_test/spectest"
 	"github.com/tetratelabs/wazero/internal/platform"
 )
@@ -14,9 +14,9 @@ func TestCompiler(t *testing.T) {
 	if !platform.CompilerSupported() {
 		t.Skip()
 	}
-	spectest.Run(t, Testcases, context.Background(), nil, compiler.NewEngine, EnabledFeatures)
+	spectest.Run(t, Testcases, context.Background(), wazero.NewRuntimeConfigCompiler().WithCoreFeatures(api.CoreFeaturesV1))
 }
 
 func TestInterpreter(t *testing.T) {
-	spectest.Run(t, Testcases, context.Background(), nil, interpreter.NewEngine, EnabledFeatures)
+	spectest.Run(t, Testcases, context.Background(), wazero.NewRuntimeConfigInterpreter().WithCoreFeatures(api.CoreFeaturesV1))
 }

internal/integration_test/spectest/v2/spec_test.go

@@ -5,9 +5,8 @@ import (
 	"embed"
 	"testing"
 
+	"github.com/tetratelabs/wazero"
 	"github.com/tetratelabs/wazero/api"
-	"github.com/tetratelabs/wazero/internal/engine/compiler"
-	"github.com/tetratelabs/wazero/internal/engine/interpreter"
 	"github.com/tetratelabs/wazero/internal/integration_test/spectest"
 	"github.com/tetratelabs/wazero/internal/platform"
 )
@@ -22,9 +21,9 @@ func TestCompiler(t *testing.T) {
 	if !platform.CompilerSupported() {
 		t.Skip()
 	}
-	spectest.Run(t, testcases, context.Background(), nil, compiler.NewEngine, enabledFeatures)
+	spectest.Run(t, testcases, context.Background(), wazero.NewRuntimeConfigCompiler().WithCoreFeatures(enabledFeatures))
 }
 
 func TestInterpreter(t *testing.T) {
-	spectest.Run(t, testcases, context.Background(), nil, interpreter.NewEngine, enabledFeatures)
+	spectest.Run(t, testcases, context.Background(), wazero.NewRuntimeConfigInterpreter().WithCoreFeatures(enabledFeatures))
 }

internal/wasm/store.go

@@ -122,11 +122,6 @@ type (
 		s *Store
 		// prev and next hold the nodes in the linked list of ModuleInstance held by Store.
 		prev, next *ModuleInstance
-		// aliases holds the module names that are aliases of this module registered in the store.
-		// Access to this field must be guarded by s.mux.
-		//
-		// Note: This is currently only used for spectests and will be nil in most cases.
-		aliases []string
 		// Source is a pointer to the Module from which this ModuleInstance derives.
 		Source *Module
 	}

internal/wasm/store_module_list.go

@@ -30,12 +30,6 @@ func (s *Store) deleteModule(m *ModuleInstance) error {
 	if m.ModuleName != "" {
 		delete(s.nameToModule, m.ModuleName)
 
-		// Under normal circumstances, m.aliases will be nil this loop will not
-		// be entered unless aliases have been created. See `*store.AliasModule`
-		for _, alias := range m.aliases {
-			delete(s.nameToModule, alias)
-		}
-
 		// Shrink the map if it's allocated more than twice the size of the list
 		newCap := len(s.nameToModule)
 		if newCap < nameToModuleShrinkThreshold {
@@ -93,27 +87,6 @@ func (s *Store) registerModule(m *ModuleInstance) error {
 	return nil
 }
 
-// AliasModule aliases the instantiated module named `src` as `dst`.
-//
-// Note: This is only used for spectests.
-func (s *Store) AliasModule(src, dst string) error {
-	s.mux.Lock()
-	defer s.mux.Unlock()
-	if _, ok := s.nameToModule[dst]; ok {
-		return nil
-	}
-	m, ok := s.nameToModule[src]
-	if !ok {
-		return nil
-	}
-	m.aliases = append(m.aliases, dst)
-	s.nameToModule[dst] = m
-	if len(s.nameToModule) > s.nameToModuleCap {
-		s.nameToModuleCap = len(s.nameToModule)
-	}
-	return nil
-}
-
 // Module implements wazero.Runtime Module
 func (s *Store) Module(moduleName string) api.Module {
 	m, err := s.module(moduleName)

internal/wasm/store_module_list_test.go

@@ -107,31 +107,6 @@ func TestStore_module(t *testing.T) {
 	})
 }
 
-func TestStore_AliasModule(t *testing.T) {
-	t.Run("alias module", func(t *testing.T) {
-		s := newStore()
-		m1 := &ModuleInstance{ModuleName: "m1"}
-		s.nameToModule[m1.ModuleName] = m1
-
-		require.NoError(t, s.AliasModule("m1", "m2"))
-		require.Equal(t, map[string]*ModuleInstance{"m1": m1, "m2": m1}, s.nameToModule)
-		// Doesn't affect module names
-		require.Nil(t, s.moduleList)
-		require.Equal(t, nameToModuleShrinkThreshold, s.nameToModuleCap)
-	})
-
-	t.Run("delete aliased module", func(t *testing.T) {
-		s := newStore()
-		m1 := &ModuleInstance{ModuleName: "m1"}
-		s.nameToModule[m1.ModuleName] = m1
-
-		require.NoError(t, s.AliasModule("m1", "m2"))
-		require.NoError(t, s.deleteModule(m1))
-		_, ok := s.nameToModule["m2"]
-		require.False(t, ok)
-	})
-}
-
 func TestStore_nameToModuleCap(t *testing.T) {
 	t.Run("nameToModuleCap grows beyond initial cap", func(t *testing.T) {
 		s := newStore()