*: add tests to statefetcher and statesync module #3918

AliceInHunterland · 2025-05-29T15:29:40Z

pkg/core/statesync/neotest_test.go

codecov · 2025-06-05T14:55:32Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.05%. Comparing base (05163de) to head (bc4deb5).
Report is 8 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3918      +/-   ##
==========================================
+ Coverage   81.78%   82.05%   +0.27%     
==========================================
  Files         345      345              
  Lines       48921    48933      +12     
==========================================
+ Hits        40010    40153     +143     
+ Misses       7242     7107     -135     
- Partials     1669     1673       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

pkg/core/statesync/neotest_test.go

AnnaShaleva · 2025-06-16T15:27:11Z

pkg/services/statefetcher/statefetcher_test.go

+			},
+		}
+		_, err := New(ledger, cfg, 0, logger, shutdown)
+		require.Error(t, err)


Check error text or exact error here and below.

pkg/services/statefetcher/statefetcher_test.go

AnnaShaleva · 2025-06-19T09:29:46Z

pkg/core/statesync/neotest_test.go

+					if err != nil {
+						return false
+					}


Remove it, it's useless after require.NoError check.

AnnaShaleva · 2025-06-19T09:32:18Z

pkg/core/statesync/neotest_test.go

+				if i > syncPoint {
+					break
+				}


Unify this condition with cycle constraint.

AnnaShaleva · 2025-06-19T09:34:18Z

pkg/core/statesync/neotest_test.go

+			require.Equal(t, 2, calls)
+			require.True(t, module.IsActive())
+			// AddMPTNodes or AddContractStorageItems up to P → storageSynced
+			if cc.name == "MPT-based" {


It's a bad pattern, don't bind it to test name. Pass mode as a parameter to checking function, see how it is done in TestStateSyncModule_RestoreBasicChain for example.

AnnaShaleva · 2025-06-19T09:35:39Z

pkg/core/statesync/neotest_test.go

+					unknown := module.GetUnknownMPTNodesBatch(1)
+					if len(unknown) == 0 {
+						break
+					}


Grap large batches, 1is too inefficient, and it's not the purpose of this test to get one-slot batches.

AnnaShaleva · 2025-06-19T09:37:52Z

pkg/core/statesync/neotest_test.go

+				require.False(t, module.NeedStorageData())
+				require.Equal(t, bcBolt.GetConfig(), module.GetConfig())
+			})
+			if cc.name == "MPT-based" {


AnnaShaleva · 2025-06-19T09:45:10Z

pkg/core/statesync/neotest_test.go

 	})
-
-	t.Run("initialized: no previous state sync point", func(t *testing.T) {
+	t.Run("pre-init: MPT-based AddContractStorageItems panic", func(t *testing.T) {


No need to duplicate tests. This functionality is already covered in TestStateSyncModule_RestoreBasicChain.

AnnaShaleva · 2025-06-19T09:45:17Z

pkg/core/statesync/neotest_test.go

 	})

-	t.Run("error: outdated state sync point in the storage", func(t *testing.T) {
-		bcBolt, _, _ := chain.NewMultiWithCustomConfig(t, boltCfg)
+	t.Run("pre-init: storage-based AddContractStorageItems error", func(t *testing.T) {


Ditto here and two methods below.

AnnaShaleva · 2025-06-19T09:46:50Z

pkg/core/statesync/neotest_test.go

+			t.Run("inactive: spout chain is too low to start state sync process", func(t *testing.T) {
+				bcBolt, _, _ := chain.NewMultiWithCustomConfig(t, cc.cfg)
+				module := bcBolt.GetStateSyncModule()
+				require.NoError(t, module.Init(uint32(2*stateSyncInterval-1)))
+				require.False(t, module.IsActive())
+				require.Equal(t, bcBolt.GetConfig(), module.GetConfig())


This test is not contract storage specific, move it out of the loop, to the place where it was before.

AnnaShaleva · 2025-06-19T09:47:29Z

pkg/core/statesync/neotest_test.go

+			t.Run("inactive: bolt chain height is close enough to spout chain height", func(t *testing.T) {
+				bcBolt, _, _ := chain.NewMultiWithCustomConfig(t, cc.cfg)
+				for i := uint32(1); i < bcSpout.BlockHeight()-stateSyncInterval; i++ {
+					b, err := bcSpout.GetBlock(bcSpout.GetHeaderHash(i))
+					require.NoError(t, err)
+					require.NoError(t, bcBolt.AddBlock(b))
+				}
+				module := bcBolt.GetStateSyncModule()
+				require.NoError(t, module.Init(bcSpout.BlockHeight()))
+				require.False(t, module.IsActive())
+				require.Equal(t, bcBolt.GetConfig(), module.GetConfig())


As far as this one.

AnnaShaleva · 2025-06-19T09:50:55Z

pkg/core/statesync/neotest_test.go

-		require.False(t, module.NeedStorageData())
-		require.True(t, module.NeedBlocks())
-		require.Equal(t, uint32(stateSyncPoint-stateSyncInterval-1), module.BlockHeight())
+			t.Run("error: bolt chain is too low to start state sync process", func(t *testing.T) {


Ditto. What I need for TestStateSyncModule_Init is to extend only those part that is related to contract storage sync initialization, that's it. We need to cover newly-added code of (s *Module) Init. The rest part of this test should be kept unchanged, I expect as less changes in git diff as possible for this test.

AnnaShaleva

Some newly-added code of pkg/core/statesync/module.go is still uncovered (error cases mostly), see https://app.codecov.io/gh/nspcc-dev/neo-go/commit/6c94a36a20bd40209dadf123a03b273359c89977/indirect-changes?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr%20comments&utm_term=nspcc-dev

AnnaShaleva · 2025-06-19T13:14:51Z

pkg/core/statesync/neotest_test.go

 	}

 	t.Run("inactive: spout chain is too low to start state sync process", func(t *testing.T) {
 		bcBolt, _, _ := chain.NewMultiWithCustomConfig(t, boltCfg)
 		module := bcBolt.GetStateSyncModule()
 		require.NoError(t, module.Init(uint32(2*stateSyncInterval-1)))
 		require.False(t, module.IsActive())
+		require.Equal(t, bcBolt.GetConfig(), module.GetConfig())


Here and below, remove this check, it's useless. module is derived from bcBolt.

module.GetConfig() is not covered by tests.

Create a separate unit-test for it. Because it's immutable field in fact, and I see no point in checking it again and again, especially in the integration test.

pkg/core/statesync/neotest_test.go

AnnaShaleva

It's important to maintain statesync tests in a proper state, because this code is not a hot-path and it's easy to break it with some new changes. That's why I'm asking to be strict and precise with these checks.

pkg/core/statesync/neotest_test.go

Close #3906 Signed-off-by: Ekaterina Pavlova <ekt@morphbits.io>

AnnaShaleva reviewed Jun 3, 2025

View reviewed changes

pkg/core/statesync/neotest_test.go Outdated Show resolved Hide resolved

AliceInHunterland force-pushed the tests branch from 0590c72 to e3fd586 Compare June 5, 2025 14:45

AliceInHunterland requested a review from AnnaShaleva June 5, 2025 14:45

AliceInHunterland marked this pull request as ready for review June 5, 2025 14:45

AliceInHunterland requested a review from roman-khimov as a code owner June 5, 2025 14:45

AnnaShaleva reviewed Jun 16, 2025

View reviewed changes

AliceInHunterland force-pushed the tests branch from e3fd586 to 2351b16 Compare June 19, 2025 08:57

AliceInHunterland requested a review from AnnaShaleva June 19, 2025 08:59

AnnaShaleva reviewed Jun 19, 2025

View reviewed changes

AliceInHunterland force-pushed the tests branch from 2351b16 to 6c94a36 Compare June 19, 2025 10:19

AliceInHunterland requested a review from AnnaShaleva June 19, 2025 10:20

AnnaShaleva reviewed Jun 19, 2025

View reviewed changes

AliceInHunterland force-pushed the tests branch from 6c94a36 to ed0f646 Compare June 20, 2025 07:25

AliceInHunterland requested a review from AnnaShaleva June 20, 2025 07:33

AnnaShaleva reviewed Jun 20, 2025

View reviewed changes

*: add tests to statefetcher and statesync module

bc4deb5

Close #3906 Signed-off-by: Ekaterina Pavlova <ekt@morphbits.io>

AliceInHunterland force-pushed the tests branch from ed0f646 to bc4deb5 Compare June 20, 2025 12:37

AliceInHunterland requested a review from AnnaShaleva June 20, 2025 12:37

AnnaShaleva approved these changes Jun 20, 2025

View reviewed changes

AnnaShaleva merged commit 6cd4fd7 into master Jun 20, 2025
33 of 34 checks passed

AnnaShaleva deleted the tests branch June 20, 2025 13:52

AnnaShaleva mentioned this pull request Jun 20, 2025

WS-based RPC server tests fail on Windows #3378

Open

*: add tests to statefetcher and statesync module #3918

*: add tests to statefetcher and statesync module #3918

Uh oh!

Conversation

AliceInHunterland commented May 29, 2025

Uh oh!

Uh oh!

codecov bot commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AnnaShaleva left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AnnaShaleva left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Jun 5, 2025 •

edited

Loading

AnnaShaleva left a comment •

edited

Loading