Merge pull request #3813 from onflow/supun/refactor-tests

SupunS · web-flow · commit e6f27ac890d6 · 2025-03-12T14:54:45.000-07:00
Run existing interpreter tests with the compiler and VM
diff --git a/Makefile b/Makefile
@@ -60,7 +60,7 @@ build-compatibility-check:
 	(cd ./tools/compatibility-check && go build .)
 
 .PHONY: ci
-ci:
+ci: test-with-compiler test-tools
 	# test all packages
 	go test -coverprofile=coverage.txt -covermode=atomic -parallel 8 -race -coverpkg $(COVERPKGS) ./...
 	# run interpreter smoke tests. results from run above are reused, so no tests runs are duplicated
@@ -69,7 +69,7 @@ ci:
 	sed -i -e 's/^.* 0 0$$//' coverage.txt
 
 .PHONY: test
-test: test-all-packages test-tools
+test: test-all-packages test-tools test-with-compiler
 
 .PHONY: test-all-packages
 test-all-packages:
@@ -82,6 +82,12 @@ test-tools:
 	(cd ./tools/constructorcheck && go test -parallel 8 ./)
 	(cd ./tools/maprange && go test -parallel 8 ./)
 
+
+.PHONY: test-with-compiler
+test-with-compiler:
+	(go test -parallel 8 ./interpreter/... -compile=true)
+
+
 .PHONY: lint-github-actions
 lint-github-actions: build-linter
 	tools/golangci-lint/golangci-lint run --out-format=colored-line-number,github-actions --timeout=5m  -v ./...
diff --git a/bbq/vm/config.go b/bbq/vm/config.go
@@ -75,7 +75,7 @@ func (c *Config) WithAccountHandler(handler stdlib.AccountHandler) *Config {
 }
 
 // TODO: This is temporary. Remove once storing/reading is supported for VM values.
-func (c *Config) interpreter() *interpreter.Interpreter {
+func (c *Config) Interpreter() *interpreter.Interpreter {
 	if c.inter == nil {
 		inter, err := interpreter.NewInterpreter(
 			nil,
diff --git a/bbq/vm/storage.go b/bbq/vm/storage.go
@@ -53,7 +53,7 @@ func ReadStored(
 	storageDomain, _ := common.StorageDomainFromIdentifier(domain)
 
 	accountStorage := storage.GetDomainStorageMap(
-		config.interpreter(),
+		config.Interpreter(),
 		address,
 		storageDomain,
 		false,
@@ -74,7 +74,7 @@ func WriteStored(
 	value Value,
 ) (existed bool) {
 
-	inter := config.interpreter()
+	inter := config.Interpreter()
 
 	accountStorage := config.Storage.GetDomainStorageMap(inter, storageAddress, domain, true)
 	interValue := VMValueToInterpreterValue(config, value)
@@ -107,7 +107,7 @@ func StoredValueExists(
 	identifier interpreter.StorageMapKey,
 ) bool {
 	accountStorage := config.Storage.GetDomainStorageMap(
-		config.interpreter(),
+		config.Interpreter(),
 		storageAddress,
 		domain,
 		false,
diff --git a/bbq/vm/test/utils.go b/bbq/vm/test/utils.go
@@ -562,6 +562,17 @@ func compileAndInvokeWithOptions(
 	arguments ...vm.Value,
 ) (vm.Value, error) {
 
+	programVM := CompileAndPrepareToInvoke(t, code, options)
+
+	result, err := programVM.Invoke(funcName, arguments...)
+	if err == nil {
+		require.Equal(t, 0, programVM.StackSize())
+	}
+
+	return result, err
+}
+
+func CompileAndPrepareToInvoke(t testing.TB, code string, options CompilerAndVMOptions) *vm.VM {
 	programs := map[common.Location]*compiledProgram{}
 
 	location := common.ScriptLocation{0x1}
@@ -600,13 +611,7 @@ func compileAndInvokeWithOptions(
 		program,
 		vmConfig,
 	)
-
-	result, err := programVM.Invoke(funcName, arguments...)
-	if err == nil {
-		require.Equal(t, 0, programVM.StackSize())
-	}
-
-	return result, err
+	return programVM
 }
 
 func compileAndInvokeWithOptionsAndPrograms(
diff --git a/bbq/vm/types.go b/bbq/vm/types.go
@@ -25,7 +25,7 @@ import (
 
 func IsSubType(config *Config, sourceType, targetType bbq.StaticType) bool {
 	// TODO: Avoid conversion to sema types.
-	inter := config.interpreter()
+	inter := config.Interpreter()
 	return inter.IsSubType(sourceType, targetType)
 }
 
diff --git a/bbq/vm/value_account.go b/bbq/vm/value_account.go
@@ -259,7 +259,7 @@ func checkAndIssueStorageCapabilityControllerWithType(
 	borrowType, ok := ty.(*interpreter.ReferenceStaticType)
 	if !ok {
 		// TODO: remove conversion. se static type in error
-		semaType, err := config.interpreter().ConvertStaticToSemaType(ty)
+		semaType, err := config.Interpreter().ConvertStaticToSemaType(ty)
 		if err != nil {
 			panic(err)
 		}
@@ -376,7 +376,7 @@ func recordStorageCapabilityController(
 	storageMapKey := interpreter.StringStorageMapKey(identifier)
 
 	accountStorage := config.Storage.GetDomainStorageMap(
-		config.interpreter(),
+		config.Interpreter(),
 		address,
 		common.StorageDomainPathCapability,
 		true,
@@ -396,7 +396,7 @@ func recordStorageCapabilityController(
 			setValue,
 		)
 		capabilityIDSetInterValue := VMValueToInterpreterValue(config, capabilityIDSet)
-		accountStorage.SetValue(config.interpreter(), storageMapKey, capabilityIDSetInterValue)
+		accountStorage.SetValue(config.Interpreter(), storageMapKey, capabilityIDSetInterValue)
 	} else {
 		capabilityIDSet := readValue.(*DictionaryValue)
 		existing := capabilityIDSet.Insert(config, setKey, setValue)
diff --git a/bbq/vm/value_capability.go b/bbq/vm/value_capability.go
@@ -211,7 +211,7 @@ func getCapabilityController(
 	storageMapKey := interpreter.Uint64StorageMapKey(capabilityID)
 
 	accountStorage := config.Storage.GetDomainStorageMap(
-		config.interpreter(),
+		config.Interpreter(),
 		address,
 		common.StorageDomainCapabilityController,
 		false,
diff --git a/bbq/vm/value_composite.go b/bbq/vm/value_composite.go
@@ -139,7 +139,7 @@ func (v *CompositeValue) SetMember(config *Config, name string, value Value) {
 	}
 
 	if existingStorable != nil {
-		inter := config.interpreter()
+		inter := config.Interpreter()
 		existingValue := interpreter.StoredValue(nil, existingStorable, config.Storage)
 
 		existingValue.DeepRemove(inter, true) // existingValue is standalone because it was overwritten in parent container.
diff --git a/bbq/vm/value_conversions.go b/bbq/vm/value_conversions.go
@@ -173,7 +173,7 @@ func VMValueToInterpreterValue(config *Config, value Value) interpreter.Value {
 			nil,
 		)
 	case *StorageReferenceValue:
-		inter := config.interpreter()
+		inter := config.Interpreter()
 		semaBorrowType, err := inter.ConvertStaticToSemaType(value.BorrowedType)
 		if err != nil {
 			panic(err)
diff --git a/bbq/vm/value_dictionary.go b/bbq/vm/value_dictionary.go
@@ -433,7 +433,7 @@ func (v *DictionaryValue) iterate(
 
 func newValueComparator(conf *Config) atree.ValueComparator {
 	return func(storage atree.SlabStorage, atreeValue atree.Value, otherStorable atree.Storable) (bool, error) {
-		inter := conf.interpreter()
+		inter := conf.Interpreter()
 		locationRange := interpreter.EmptyLocationRange
 		value := interpreter.MustConvertStoredValue(inter, atreeValue)
 		otherValue := interpreter.StoredValue(inter, otherStorable, storage)
@@ -443,7 +443,7 @@ func newValueComparator(conf *Config) atree.ValueComparator {
 
 func newHashInputProvider(conf *Config) atree.HashInputProvider {
 	return func(value atree.Value, scratch []byte) ([]byte, error) {
-		inter := conf.interpreter()
+		inter := conf.Interpreter()
 		locationRange := interpreter.EmptyLocationRange
 		hashInput := interpreter.MustConvertStoredValue(inter, value).(interpreter.HashableValue).
 			HashInput(inter, locationRange, scratch)
diff --git a/interpreter/interpreter_test.go b/interpreter/interpreter_test.go
@@ -19,6 +19,7 @@
 package interpreter_test
 
 import (
+	"flag"
 	"fmt"
 	"testing"
 
@@ -27,8 +28,16 @@ import (
 	"github.com/onflow/cadence/common"
 	. "github.com/onflow/cadence/interpreter"
 	"github.com/onflow/cadence/sema"
+	"github.com/onflow/cadence/test_utils"
 )
 
+var compile = flag.Bool("compile", false, "Run tests using the compiler")
+
+func parseCheckAndPrepare(tb testing.TB, code string) test_utils.Invokable {
+	tb.Helper()
+	return test_utils.ParseCheckAndPrepare(tb, code, *compile)
+}
+
 func TestInterpreterOptionalBoxing(t *testing.T) {
 
 	t.Parallel()
diff --git a/interpreter/while_test.go b/interpreter/while_test.go
@@ -31,7 +31,7 @@ func TestInterpretWhileStatement(t *testing.T) {
 
 	t.Parallel()
 
-	inter := parseCheckAndInterpret(t, `
+	invokable := parseCheckAndPrepare(t, `
        fun test(): Int {
            var x = 0
            while x < 5 {
@@ -42,12 +42,12 @@ func TestInterpretWhileStatement(t *testing.T) {
 
     `)
 
-	value, err := inter.Invoke("test")
+	value, err := invokable.Invoke("test")
 	require.NoError(t, err)
 
 	AssertValuesEqual(
 		t,
-		inter,
+		invokable,
 		interpreter.NewUnmeteredIntValueFromInt64(6),
 		value,
 	)
@@ -57,7 +57,7 @@ func TestInterpretWhileStatementWithReturn(t *testing.T) {
 
 	t.Parallel()
 
-	inter := parseCheckAndInterpret(t, `
+	invokable := parseCheckAndPrepare(t, `
        fun test(): Int {
            var x = 0
            while x < 10 {
@@ -70,12 +70,12 @@ func TestInterpretWhileStatementWithReturn(t *testing.T) {
        }
     `)
 
-	value, err := inter.Invoke("test")
+	value, err := invokable.Invoke("test")
 	require.NoError(t, err)
 
 	AssertValuesEqual(
 		t,
-		inter,
+		invokable,
 		interpreter.NewUnmeteredIntValueFromInt64(6),
 		value,
 	)
@@ -85,7 +85,7 @@ func TestInterpretWhileStatementWithContinue(t *testing.T) {
 
 	t.Parallel()
 
-	inter := parseCheckAndInterpret(t, `
+	invokable := parseCheckAndPrepare(t, `
        fun test(): Int {
            var i = 0
            var x = 0
@@ -100,12 +100,12 @@ func TestInterpretWhileStatementWithContinue(t *testing.T) {
        }
     `)
 
-	value, err := inter.Invoke("test")
+	value, err := invokable.Invoke("test")
 	require.NoError(t, err)
 
 	AssertValuesEqual(
 		t,
-		inter,
+		invokable,
 		interpreter.NewUnmeteredIntValueFromInt64(6),
 		value,
 	)
@@ -115,7 +115,7 @@ func TestInterpretWhileStatementWithBreak(t *testing.T) {
 
 	t.Parallel()
 
-	inter := parseCheckAndInterpret(t, `
+	invokable := parseCheckAndPrepare(t, `
        fun test(): Int {
            var x = 0
            while x < 10 {
@@ -128,12 +128,12 @@ func TestInterpretWhileStatementWithBreak(t *testing.T) {
        }
     `)
 
-	value, err := inter.Invoke("test")
+	value, err := invokable.Invoke("test")
 	require.NoError(t, err)
 
 	AssertValuesEqual(
 		t,
-		inter,
+		invokable,
 		interpreter.NewUnmeteredIntValueFromInt64(5),
 		value,
 	)
diff --git a/test_utils/interpreter_utils/values.go b/test_utils/interpreter_utils/values.go
@@ -49,20 +49,20 @@ func AssertValueSlicesEqual(t testing.TB, inter *interpreter.Interpreter, expect
 	return true
 }
 
-func ValuesAreEqual(inter *interpreter.Interpreter, expected, actual interpreter.Value) bool {
+func ValuesAreEqual(context interpreter.ValueComparisonContext, expected, actual interpreter.Value) bool {
 	if expected == nil {
 		return actual == nil
 	}
 
 	if expected, ok := expected.(interpreter.EquatableValue); ok {
-		return expected.Equal(inter, interpreter.EmptyLocationRange, actual)
+		return expected.Equal(context, interpreter.EmptyLocationRange, actual)
 	}
 
 	return assert.ObjectsAreEqual(expected, actual)
 }
 
-func AssertValuesEqual(t testing.TB, interpreter *interpreter.Interpreter, expected, actual interpreter.Value) bool {
-	if !ValuesAreEqual(interpreter, expected, actual) {
+func AssertValuesEqual(t testing.TB, context interpreter.ValueComparisonContext, expected, actual interpreter.Value) bool {
+	if !ValuesAreEqual(context, expected, actual) {
 		diff := pretty.Diff(expected, actual)
 
 		var message string
diff --git a/test_utils/test_utils.go b/test_utils/test_utils.go

Original file line number	Diff line number	Diff line change
`@@ -75,7 +75,7 @@ func (c Config) WithAccountHandler(handler stdlib.AccountHandler) Config {`
`75`	`75`	`}`
`76`	`76`
`77`	`77`	`// TODO: This is temporary. Remove once storing/reading is supported for VM values.`
`78`		`-func (c Config) interpreter() interpreter.Interpreter {`
	`78`	`+func (c Config) Interpreter() interpreter.Interpreter {`
`79`	`79`	`if c.inter == nil {`
`80`	`80`	`inter, err := interpreter.NewInterpreter(`
`81`	`81`	`nil,`
Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,7 @@ import (`
`25`	`25`
`26`	`26`	`func IsSubType(config *Config, sourceType, targetType bbq.StaticType) bool {`
`27`	`27`	`// TODO: Avoid conversion to sema types.`
`28`		`- inter := config.interpreter()`
	`28`	`+ inter := config.Interpreter()`
`29`	`29`	`return inter.IsSubType(sourceType, targetType)`
`30`	`30`	`}`
`31`	`31`
Original file line number	Diff line number	Diff line change
`@@ -139,7 +139,7 @@ func (v CompositeValue) SetMember(config Config, name string, value Value) {`
`139`	`139`	`}`
`140`	`140`
`141`	`141`	`if existingStorable != nil {`
`142`		`- inter := config.interpreter()`
	`142`	`+ inter := config.Interpreter()`
`143`	`143`	`existingValue := interpreter.StoredValue(nil, existingStorable, config.Storage)`
`144`	`144`
`145`	`145`	`existingValue.DeepRemove(inter, true) // existingValue is standalone because it was overwritten in parent container.`
Original file line number	Diff line number	Diff line change
`@@ -173,7 +173,7 @@ func VMValueToInterpreterValue(config *Config, value Value) interpreter.Value {`
`173`	`173`	`nil,`
`174`	`174`	`)`
`175`	`175`	`case *StorageReferenceValue:`
`176`		`- inter := config.interpreter()`
	`176`	`+ inter := config.Interpreter()`
`177`	`177`	`semaBorrowType, err := inter.ConvertStaticToSemaType(value.BorrowedType)`
`178`	`178`	`if err != nil {`
`179`	`179`	`panic(err)`