Merge pull request #3144 from onflow/bastian/improve-migration-2

Improve migrations

migrations/capcons/migration_test.go

@@ -514,16 +514,12 @@ func testPathCapabilityValueMigration(
 	err = migration.Commit()
 	require.NoError(t, err)
 
-	// Check migrated capabilities
+	// Assert
 
 	assert.Equal(t,
 		expectedMigrations,
 		reporter.migrations,
 	)
-	assert.Equal(t,
-		expectedErrors,
-		reporter.errors,
-	)
 	assert.Equal(t,
 		expectedPathMigrations,
 		reporter.pathCapabilityMigrations,
@@ -532,6 +528,13 @@ func testPathCapabilityValueMigration(
 		expectedMissingCapabilityIDs,
 		reporter.missingCapabilityIDs,
 	)
+	require.Equal(t,
+		expectedErrors,
+		reporter.errors,
+	)
+
+	err = storage.CheckHealth()
+	require.NoError(t, err)
 
 	if len(expectedMissingCapabilityIDs) == 0 {
 
@@ -1354,10 +1357,6 @@ func testLinkMigration(
 		expectedMigrations,
 		reporter.migrations,
 	)
-	assert.Empty(t,
-		expectedErrors,
-		reporter.errors,
-	)
 	assert.Equal(t,
 		expectedLinkMigrations,
 		reporter.linkMigrations,
@@ -1370,6 +1369,13 @@ func testLinkMigration(
 		expectedMissingTargets,
 		reporter.missingTargets,
 	)
+	require.Equal(t,
+		expectedErrors,
+		reporter.errors,
+	)
+
+	err = storage.CheckHealth()
+	require.NoError(t, err)
 }
 
 func TestLinkMigration(t *testing.T) {
@@ -2073,20 +2079,22 @@ func TestPublishedPathCapabilityValueMigration(t *testing.T) {
 	err = migration.Commit()
 	require.NoError(t, err)
 
-	// Check migrated capabilities
+	// Assert
 
 	assert.Equal(t,
 		expectedMigrations,
 		reporter.migrations,
 	)
-	assert.Empty(t, reporter.errors)
 	assert.Equal(t,
 		expectedPathMigrations,
 		reporter.pathCapabilityMigrations,
 	)
 	require.Nil(t, reporter.missingCapabilityIDs)
 
-	// Check
+	require.Empty(t, reporter.errors)
+
+	err = storage.CheckHealth()
+	require.NoError(t, err)
 
 	// language=cadence
 	checkScript := `
@@ -2320,7 +2328,7 @@ func TestUntypedPathCapabilityValueMigration(t *testing.T) {
 	err = migration.Commit()
 	require.NoError(t, err)
 
-	// Check migrated capabilities
+	// Assert
 
 	assert.Equal(t,
 		expectedMigrations,
@@ -2331,8 +2339,12 @@ func TestUntypedPathCapabilityValueMigration(t *testing.T) {
 		reporter.pathCapabilityMigrations,
 	)
 	require.Nil(t, reporter.missingCapabilityIDs)
+
 	require.Empty(t, reporter.errors)
 
+	err = storage.CheckHealth()
+	require.NoError(t, err)
+
 	// Check
 
 	// language=cadence

migrations/entitlements/migration.go

@@ -241,16 +241,10 @@ func ConvertValueToEntitlements(
 			return nil, nil
 		}
 
-		iterator := v.Iterator(inter, interpreter.EmptyLocationRange)
-
-		return interpreter.NewArrayValueWithIterator(
+		return v.NewWithType(
 			inter,
+			interpreter.EmptyLocationRange,
 			entitledElementType.(interpreter.ArrayStaticType),
-			v.GetOwner(),
-			uint64(v.Count()),
-			func() interpreter.Value {
-				return iterator.Next(inter, interpreter.EmptyLocationRange)
-			},
 		), nil
 
 	case *interpreter.DictionaryValue:
@@ -261,29 +255,16 @@ func ConvertValueToEntitlements(
 			return nil, err
 		}
 
-		if entitledElementType != nil {
-			var keysAndValues []interpreter.Value
-
-			iterator := v.Iterator()
-			for {
-				keyValue, value := iterator.Next(inter)
-				if keyValue == nil {
-					break
-				}
-
-				keysAndValues = append(keysAndValues, keyValue)
-				keysAndValues = append(keysAndValues, value)
-			}
-
-			return interpreter.NewDictionaryValueWithAddress(
-				inter,
-				interpreter.EmptyLocationRange,
-				entitledElementType.(*interpreter.DictionaryStaticType),
-				v.GetOwner(),
-				keysAndValues...,
-			), nil
+		if entitledElementType == nil {
+			return nil, nil
 		}
 
+		return v.NewWithType(
+			inter,
+			interpreter.EmptyLocationRange,
+			entitledElementType.(*interpreter.DictionaryStaticType),
+		), nil
+
 	case *interpreter.IDCapabilityValue:
 		borrowType := v.BorrowType
 

migrations/entitlements/migration_test.go

@@ -725,7 +725,10 @@ func convertEntireTestValue(
 
 	// Assert
 
-	assert.Len(t, reporter.errors, 0)
+	require.Empty(t, reporter.errors)
+
+	err = storage.CheckHealth()
+	require.NoError(t, err)
 
 	if migratedValue == nil {
 		return v
@@ -1317,8 +1320,12 @@ func TestConvertToEntitledValue(t *testing.T) {
 		},
 	}
 
-	test := func(t *testing.T, testCase testCase, valueGenerator valueGenerator, typeGenerator typeGenerator) {
-
+	test := func(
+		t *testing.T,
+		testCase testCase,
+		valueGenerator valueGenerator,
+		typeGenerator typeGenerator,
+	) {
 		input := valueGenerator.wrap(typeGenerator.wrap(testCase.Input))
 		if input == nil {
 			return
@@ -1328,6 +1335,9 @@ func TestConvertToEntitledValue(t *testing.T) {
 
 		convertedValue := convertEntireTestValue(t, inter, storage, testAddress, input)
 
+		err := storage.CheckHealth()
+		require.NoError(t, err)
+
 		switch convertedValue := convertedValue.(type) {
 		case interpreter.EquatableValue:
 			require.True(t,
@@ -1488,11 +1498,20 @@ func TestMigrateSimpleContract(t *testing.T) {
 	}
 
 	for name, testCase := range testCases {
+		transferredValue := testCase.storedValue.Transfer(
+			inter,
+			interpreter.EmptyLocationRange,
+			atree.Address(account),
+			false,
+			nil,
+			nil,
+		)
+
 		inter.WriteStored(
 			account,
 			storageIdentifier,
 			interpreter.StringStorageMapKey(name),
-			testCase.storedValue,
+			transferredValue,
 		)
 	}
 
@@ -1521,7 +1540,10 @@ func TestMigrateSimpleContract(t *testing.T) {
 
 	// Assert
 
-	assert.Len(t, reporter.errors, 0)
+	require.Empty(t, reporter.errors)
+
+	err = storage.CheckHealth()
+	require.NoError(t, err)
 
 	storageMap := storage.GetStorageMap(account, storageIdentifier, false)
 	require.NotNil(t, storageMap)
@@ -1705,7 +1727,11 @@ func TestMigratePublishedValue(t *testing.T) {
 
 	// Assert
 
-	assert.Len(t, reporter.errors, 0)
+	require.Empty(t, reporter.errors)
+
+	err = storage.CheckHealth()
+	require.NoError(t, err)
+
 	assert.Equal(t,
 		map[struct {
 			interpreter.StorageKey
@@ -1959,7 +1985,11 @@ func TestMigratePublishedValueAcrossTwoAccounts(t *testing.T) {
 
 	// Assert
 
-	assert.Len(t, reporter.errors, 0)
+	require.Empty(t, reporter.errors)
+
+	err = storage.CheckHealth()
+	require.NoError(t, err)
+
 	assert.Equal(t,
 		map[struct {
 			interpreter.StorageKey
@@ -2408,7 +2438,11 @@ func TestMigrateArrayOfValues(t *testing.T) {
 
 	// Assert
 
-	assert.Len(t, reporter.errors, 0)
+	require.Empty(t, reporter.errors)
+
+	err = storage.CheckHealth()
+	require.NoError(t, err)
+
 	assert.Equal(t,
 		map[struct {
 			interpreter.StorageKey
@@ -2655,7 +2689,11 @@ func TestMigrateDictOfValues(t *testing.T) {
 
 	// Assert
 
-	assert.Len(t, reporter.errors, 0)
+	require.Empty(t, reporter.errors)
+
+	err = storage.CheckHealth()
+	require.NoError(t, err)
+
 	assert.Equal(t,
 		map[struct {
 			interpreter.StorageKey
@@ -2974,7 +3012,11 @@ func TestMigrateCapConsAcrossTwoAccounts(t *testing.T) {
 
 	// Assert
 
-	assert.Len(t, reporter.errors, 0)
+	require.Empty(t, reporter.errors)
+
+	err = storage.CheckHealth()
+	require.NoError(t, err)
+
 	assert.Len(t, reporter.migrated, 1)
 
 	// TODO: assert
@@ -3063,7 +3105,8 @@ func TestRehash(t *testing.T) {
 			nil,
 			utils.TestLocation,
 			&interpreter.Config{
-				Storage:                       storage,
+				Storage: storage,
+				// NOTE: disabled, because encoded and decoded values are expected to not match
 				AtreeValueValidationEnabled:   false,
 				AtreeStorageValidationEnabled: true,
 			},
@@ -3154,6 +3197,9 @@ func TestRehash(t *testing.T) {
 
 		err := storage.Commit(inter, false)
 		require.NoError(t, err)
+
+		err = storage.CheckHealth()
+		require.NoError(t, err)
 	})
 
 	t.Run("migrate", func(t *testing.T) {
@@ -3200,6 +3246,13 @@ func TestRehash(t *testing.T) {
 		err := migration.Commit()
 		require.NoError(t, err)
 
+		// Assert
+
+		err = storage.CheckHealth()
+		require.NoError(t, err)
+
+		assert.Empty(t, reporter.errors)
+
 		require.Equal(t,
 			map[struct {
 				interpreter.StorageKey
@@ -3221,6 +3274,9 @@ func TestRehash(t *testing.T) {
 
 		storage, inter := newStorageAndInterpreter(t)
 
+		err := storage.CheckHealth()
+		require.NoError(t, err)
+
 		storageMap := storage.GetStorageMap(
 			testAddress,
 			common.PathDomainStorage.Identifier(),