Skip to content

Commit

Permalink
Prevent migration tests from using out-of-sync data
Browse files Browse the repository at this point in the history
Currently, some migration tests reuse storage for migration.
It can lead to tests failing when the storage cache gets
out of sync with underlying storage.

For example:
1. Use storage 1 to store valueA in public domain directly.
2. Use storage 2 to store valueB in public domain via
   ExecuteTransaction().
3. Reuse storage 1 for migration.  Here, migration
   only sees valueA from cache instead of both values.

This commit prevents out-of-sync issues by creating new
runtime.Storage for migrations instead of reusing old storage.
  • Loading branch information
fxamacker committed Oct 2, 2024
1 parent ed99fbf commit 88c3cf0
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 28 deletions.
85 changes: 58 additions & 27 deletions migrations/capcons/migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -511,18 +511,20 @@ func testPathCapabilityValueMigration(
)

// Create and store path and account links
{
// Create new runtime.Storage used for storing path and account links
storage, inter, err := rt.Storage(runtime.Context{
Interface: runtimeInterface,
})
require.NoError(t, err)

storage, inter, err := rt.Storage(runtime.Context{
Interface: runtimeInterface,
})
require.NoError(t, err)
storeTestPathLinks(t, pathLinks, storage, inter)

storeTestPathLinks(t, pathLinks, storage, inter)
storeTestAccountLinks(accountLinks, storage, inter)

storeTestAccountLinks(accountLinks, storage, inter)

err = storage.Commit(inter, false)
require.NoError(t, err)
err = storage.Commit(inter, false)
require.NoError(t, err)
}

// Save capability values into account

Expand Down Expand Up @@ -557,6 +559,15 @@ func testPathCapabilityValueMigration(

// Migrate

// Create new runtime.Storage for migration.
// WARNING: don't reuse old storage (created for storing path and account links)
// because it has outdated cache after ExecuteTransaction() commits new data
// to underlying ledger.
storage, inter, err := rt.Storage(runtime.Context{
Interface: runtimeInterface,
})
require.NoError(t, err)

migration, err := migrations.NewStorageMigration(inter, storage, "test", testAddress)
require.NoError(t, err)

Expand Down Expand Up @@ -2410,16 +2421,17 @@ func TestPublishedPathCapabilityValueMigration(t *testing.T) {
)

// Create and store path links
{
storage, inter, err := rt.Storage(runtime.Context{
Interface: runtimeInterface,
})
require.NoError(t, err)

storage, inter, err := rt.Storage(runtime.Context{
Interface: runtimeInterface,
})
require.NoError(t, err)

storeTestPathLinks(t, pathLinks, storage, inter)
storeTestPathLinks(t, pathLinks, storage, inter)

err = storage.Commit(inter, false)
require.NoError(t, err)
err = storage.Commit(inter, false)
require.NoError(t, err)
}

// Save capability values into account

Expand All @@ -2432,7 +2444,7 @@ func TestPublishedPathCapabilityValueMigration(t *testing.T) {
}
`

err = rt.ExecuteTransaction(
err := rt.ExecuteTransaction(
runtime.Script{
Source: []byte(setupTx),
},
Expand All @@ -2446,6 +2458,15 @@ func TestPublishedPathCapabilityValueMigration(t *testing.T) {

// Migrate

// Create new runtime.Storage for migration.
// WARNING: don't reuse old storage (created for storing path links)
// because it has outdated cache after ExecuteTransaction() commits new data
// to underlying ledger.
storage, inter, err := rt.Storage(runtime.Context{
Interface: runtimeInterface,
})
require.NoError(t, err)

migration, err := migrations.NewStorageMigration(inter, storage, "test", testAddress)
require.NoError(t, err)

Expand Down Expand Up @@ -2663,16 +2684,17 @@ func TestUntypedPathCapabilityValueMigration(t *testing.T) {
)

// Create and store path links
{
storage, inter, err := rt.Storage(runtime.Context{
Interface: runtimeInterface,
})
require.NoError(t, err)

storage, inter, err := rt.Storage(runtime.Context{
Interface: runtimeInterface,
})
require.NoError(t, err)

storeTestPathLinks(t, pathLinks, storage, inter)
storeTestPathLinks(t, pathLinks, storage, inter)

err = storage.Commit(inter, false)
require.NoError(t, err)
err = storage.Commit(inter, false)
require.NoError(t, err)
}

// Save capability values into account

Expand All @@ -2686,7 +2708,7 @@ func TestUntypedPathCapabilityValueMigration(t *testing.T) {
}
`

err = rt.ExecuteTransaction(
err := rt.ExecuteTransaction(
runtime.Script{
Source: []byte(setupTx),
},
Expand All @@ -2700,6 +2722,15 @@ func TestUntypedPathCapabilityValueMigration(t *testing.T) {

// Migrate

// Create new runtime.Storage for migration.
// WARNING: don't reuse old storage (created for storing path links)
// because it has outdated cache after ExecuteTransaction() commits new data
// to underlying ledger.
storage, inter, err := rt.Storage(runtime.Context{
Interface: runtimeInterface,
})
require.NoError(t, err)

migration, err := migrations.NewStorageMigration(inter, storage, "test", testAddress)
require.NoError(t, err)

Expand Down
2 changes: 1 addition & 1 deletion version.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 88c3cf0

Please sign in to comment.