NewSnapshotReader Nil Error

[jcastrence]

Type of change

• Improvement (improvement to code, performance, etc)

Description

• Edited NewSnapshotReader so it now logs if the passed in data file does not exist

Related issues

• FAB-18469

[manish-sethi]

My suggestion was to simply add the following code in the test file at line 535.

		t.Run("error_missing_data_file_"+f, func(t *testing.T) {
			init()
			defer cleanup()

			dataFile := filepath.Join(snapshotDir, f)
			require.NoError(t, os.Remove(dataFile))
			err := dbEnv.GetProvider().ImportFromSnapshot(
				generateLedgerID(t), version.NewHeight(10, 10), snapshotDir)
			require.Contains(t, err.Error(), fmt.Sprintf("file [%s] does not exist", dataFile))
		})

[jcastrence]

My suggestion was to simply add the following code in the test file at line 535.

		t.Run("error_missing_data_file_"+f, func(t *testing.T) {
			init()
			defer cleanup()

			dataFile := filepath.Join(snapshotDir, f)
			require.NoError(t, os.Remove(dataFile))
			err := dbEnv.GetProvider().ImportFromSnapshot(
				generateLedgerID(t), version.NewHeight(10, 10), snapshotDir)
			require.Contains(t, err.Error(), fmt.Sprintf("file [%s] does not exist", dataFile))
		})

Right, however the changes I've made to snapshot.go have caused some of the existing test cases to fail since they are missing either public state or private state hashes. Because of the addition of a new error being thrown if files are missing, they will fail since they are not expecting this new error. I thought that it'd be best to then test these test cases with missing files separately which ends up covering the addition you suggested. What are your thoughts on this?

[manish-sethi]

What are your thoughts on this?

My suggestion would be to fix the input to the existing test.

[denyeart]

Upon further discussion the nil response is expected in some scenarios (such as no private state data) so we can simply leave the functionality as-is and improve the comments and logging to make the intended use more clear.

[manish-sethi]

Suggested change

	log.Printf("Snapshot data file name does not exist")
	logger.Infow("Data file does not exist. Nothing to be done.", "filepath", dataFilePath)