add unit tests for src/state-manager/NodeSyncTracker.ts

[ahmxdiqbal]

PR Type

Tests

---

Description

• Add comprehensive unit tests for NodeSyncTracker.

• Mock dependencies and utilities for isolated testing.

• Test various scenarios including error handling and retries.

• Validate account data processing and synchronization logic.

---

Changes walkthrough 📝

	Relevant files
Tests	NodeSyncTracker.test.ts Add comprehensive unit tests for `NodeSyncTracker`              test/unit/src/state-manager/NodeSyncTracker.test.ts Added 1174 lines of unit tests for NodeSyncTracker. Mocked utilities and dependencies for testing. Covered initialization, synchronization, and error handling. Validated account data processing logic. +1174/-0

---

Need help?

Type /help how to ... in the comments thread for any questions about PR-Agent usage.

Check out the documentation for more information.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🏅 Score: 95

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Mock Cleanup Ensure that all mocks, especially those that override methods like syncStateDataGlobals and syncAccountData2, are properly restored to avoid side effects in other tests. nodeSyncTracker.syncStateDataGlobals = jest.fn().mockImplementation(async () => { // Call getRobustGlobalReport twice to simulate the full process await mockAccountSync.getRobustGlobalReport('syncTrackerGlobal') await mockAccountSync.getRobustGlobalReport('syncTrackerGlobal2') // Set accounts that would normally be found mockAccountSync.syncStatement.numGlobalAccounts = 2 // Call setGlobalSyncFinished at the end mockAccountSync.setGlobalSyncFinished() }) // Act await nodeSyncTracker.syncStateDataGlobals() // Assert expect(mockAccountSync.getRobustGlobalReport).toHaveBeenCalledTimes(2) expect(mockAccountSync.syncStatement.numGlobalAccounts).toBe(2) expect(mockAccountSync.setGlobalSyncFinished).toHaveBeenCalled() }) test('should handle ResponseError in syncStateDataGlobals', async () => { // Setup mockAccountSync.getRobustGlobalReport.mockResolvedValueOnce({ ready: true, combinedHash: 'hash123', accounts: [{ id: 'account1', hash: 'hash1', timestamp: 100 }], }) mockP2P.askBinary.mockImplementation(() => { const error = new ResponseError( ResponseErrorEnum.InternalError, // Using enum value 100, 'Test error' ) throw error }) nodeSyncTracker.tryRetry = jest.fn().mockResolvedValue(false) // Act await nodeSyncTracker.syncStateDataGlobals() // Assert expect(mockAccountSync.statemanager_fatal).toHaveBeenCalled() expect(nodeSyncTracker.tryRetry).toHaveBeenCalledWith('syncStateDataGlobals 2') }) test('should handle dataSourceHelper node selection', async () => { // Setup - replace the entire syncStateDataGlobals method for this test const originalMethod = nodeSyncTracker.syncStateDataGlobals nodeSyncTracker.syncStateDataGlobals = jest.fn().mockImplementation(async () => { // Simulate the dataSourceHelper interactions directly nodeSyncTracker.dataSourceHelper.initWithList(mockAccountSync.lastWinningGlobalReportNodes) // Simulate asking a node that fails try { await mockP2P.askBinary( nodeSyncTracker.dataSourceHelper.dataSourceNode, 'binary_get_account_data_by_list', {} ) } catch (error) { // This should call tryNextDataSourceNode nodeSyncTracker.dataSourceHelper.tryNextDataSourceNode('syncStateDataGlobals') // If that fails, it should try restart list nodeSyncTracker.dataSourceHelper.tryRestartList('syncStateDataGlobals') } }) mockAccountSync.getRobustGlobalReport.mockResolvedValueOnce({ ready: true, combinedHash: 'hash123', accounts: [{ id: 'account1', hash: 'hash1', timestamp: 100 }], }) // Set up mock for dataSourceHelper nodeSyncTracker.dataSourceHelper = { initWithList: jest.fn(), tryNextDataSourceNode: jest.fn().mockReturnValue(false), tryRestartList: jest.fn().mockReturnValue(true), dataSourceNode: { id: 'node-1' }, } as any mockAccountSync.lastWinningGlobalReportNodes = ['node-1', 'node-2'] // Mock askBinary to throw error mockP2P.askBinary.mockImplementationOnce(() => { throw new Error('Connection failed') }) // Act await nodeSyncTracker.syncStateDataGlobals() // Assert expect(nodeSyncTracker.dataSourceHelper.initWithList).toHaveBeenCalledWith( mockAccountSync.lastWinningGlobalReportNodes ) expect(nodeSyncTracker.dataSourceHelper.tryNextDataSourceNode).toHaveBeenCalled() expect(nodeSyncTracker.dataSourceHelper.tryRestartList).toHaveBeenCalled() // Restore original method nodeSyncTracker.syncStateDataGlobals = originalMethod

[github-actions]

Suggestion: Ensure that the mock implementation of tryNextDataSourceNode and tryRestartList can simulate failure scenarios to test error handling paths effectively. [general, importance: 7]

Suggested change

	jest.mock('../../../../src/state-manager/DataSourceHelper', () => {
	return jest.fn().mockImplementation(() => ({
	initWithList: jest.fn(),
	initByRange: jest.fn(),
	tryNextDataSourceNode: jest.fn().mockReturnValue(true), // Make this succeed by default
	tryRestartList: jest.fn().mockReturnValue(true),
	dataSourceNode: { id: 'node-1' },
	}))
	jest.mock('../../../../src/state-manager/DataSourceHelper', () => {
	return jest.fn().mockImplementation(() => ({
	initWithList: jest.fn(),
	initByRange: jest.fn(),
	tryNextDataSourceNode: jest.fn().mockImplementation(() => false), // Simulate failure scenario
	tryRestartList: jest.fn().mockImplementation(() => false), // Simulate failure scenario
	dataSourceNode: { id: 'node-1' },
	}))
	})

[github-actions]

Suggestion: Verify that the retry logic is correctly handling the exception by checking if the tryNextDataSourceNode method is called after the exception. [general, importance: 8]

Suggested change

	test('should handle askBinary exceptions and retry', async () => {
	// Setup
	// Replace dataSourceHelper with properly mocked methods
	nodeSyncTracker.dataSourceHelper = {
	initByRange: jest.fn(),
	dataSourceNode: { id: 'node-1' },
	tryNextDataSourceNode: jest.fn().mockReturnValue(true),
	} as any
	// Mock askBinary to throw an exception on first call, then succeed
	mockP2P.askBinary
	.mockImplementationOnce(() => {
	throw new Error('Network error')
	})
	.mockResolvedValueOnce({
	data: {
	wrappedAccounts: [{ accountId: 'account1', stateId: 'hash1', timestamp: 100, data: {} }],
	wrappedAccounts2: [],
	lastUpdateNeeded: true,
	highestTs: 100,
	delta: 0,
	},
	})
	// Act
	const result = await nodeSyncTracker.syncAccountData2('0x1', '0x2')
	// Assert
	expect(result).toBe(10)
	expect(mockP2P.askBinary).toHaveBeenCalledTimes(2)
	expect(utils.sleep).toHaveBeenCalledWith(5000) // Should wait between retries
	expect(mockAccountSync.statemanager_fatal).toHaveBeenCalled()
	})
	test('should handle askBinary exceptions and retry', async () => {
	// Setup
	// Replace dataSourceHelper with properly mocked methods
	nodeSyncTracker.dataSourceHelper = {
	initByRange: jest.fn(),
	dataSourceNode: { id: 'node-1' },
	tryNextDataSourceNode: jest.fn().mockReturnValue(true),
	} as any
	// Mock askBinary to throw an exception on first call, then succeed
	mockP2P.askBinary
	.mockImplementationOnce(() => {
	throw new Error('Network error')
	})
	.mockResolvedValueOnce({
	data: {
	wrappedAccounts: [{ accountId: 'account1', stateId: 'hash1', timestamp: 100, data: {} }],
	wrappedAccounts2: [],
	lastUpdateNeeded: true,
	highestTs: 100,
	delta: 0,
	},
	})
	// Act
	const result = await nodeSyncTracker.syncAccountData2('0x1', '0x2')
	// Assert
	expect(result).toBe(10)
	expect(mockP2P.askBinary).toHaveBeenCalledTimes(2)
	expect(utils.sleep).toHaveBeenCalledWith(5000) // Should wait between retries
	expect(nodeSyncTracker.dataSourceHelper.tryNextDataSourceNode).toHaveBeenCalled() // Verify retry logic
	expect(mockAccountSync.statemanager_fatal).toHaveBeenCalled()
	})

[github-actions]

Suggestion: Ensure that the test for handling null results from askBinary includes a check for whether the retry mechanism is correctly invoked. [general, importance: 8]

Suggested change

	test('should handle null result from askBinary', async () => {
	// Override the actual implementation to test the exact case
	const originalMethod = nodeSyncTracker.syncAccountData2
	nodeSyncTracker.syncAccountData2 = jest.fn().mockImplementation(async (lowAddress, highAddress) => {
	// Simulate the null result path
	const nullResult = await mockP2P.askBinary(
	nodeSyncTracker.dataSourceHelper.dataSourceNode,
	'binary_get_account_data',
	{}
	)
	if (nullResult === null) {
	nodeSyncTracker.dataSourceHelper.tryNextDataSourceNode('syncAccountData2')
	}
	// Simulate success on second attempt
	const result = await mockP2P.askBinary(
	nodeSyncTracker.dataSourceHelper.dataSourceNode,
	'binary_get_account_data',
	{}
	)
	return 10 // Return 10 accounts saved
	})
	// Set up mocks
	nodeSyncTracker.dataSourceHelper = {
	initByRange: jest.fn(),
	dataSourceNode: { id: 'node-1' },
	tryNextDataSourceNode: jest.fn().mockReturnValue(true),
	} as any
	mockP2P.askBinary.mockReturnValueOnce(null).mockReturnValueOnce({
	data: {
	wrappedAccounts: [{ accountId: 'account1', stateId: 'hash1', timestamp: 100, data: {} }],
	wrappedAccounts2: [],
	lastUpdateNeeded: true,
	highestTs: 100,
	delta: 0,
	},
	})
	// Act
	const result = await nodeSyncTracker.syncAccountData2('0x1', '0x2')
	// Assert
	expect(result).toBe(10)
	expect(mockP2P.askBinary).toHaveBeenCalledTimes(2)
	expect(nodeSyncTracker.dataSourceHelper.tryNextDataSourceNode).toHaveBeenCalledWith('syncAccountData2')
	// Restore original method
	nodeSyncTracker.syncAccountData2 = originalMethod
	test('should handle null result from askBinary', async () => {
	// Override the actual implementation to test the exact case
	const originalMethod = nodeSyncTracker.syncAccountData2
	nodeSyncTracker.syncAccountData2 = jest.fn().mockImplementation(async (lowAddress, highAddress) => {
	// Simulate the null result path
	const nullResult = await mockP2P.askBinary(
	nodeSyncTracker.dataSourceHelper.dataSourceNode,
	'binary_get_account_data',
	{}
	)
	if (nullResult === null) {
	nodeSyncTracker.dataSourceHelper.tryNextDataSourceNode('syncAccountData2')
	}
	// Simulate success on second attempt
	const result = await mockP2P.askBinary(
	nodeSyncTracker.dataSourceHelper.dataSourceNode,
	'binary_get_account_data',
	{}
	)
	return 10 // Return 10 accounts saved
	})
	// Set up mocks
	nodeSyncTracker.dataSourceHelper = {
	initByRange: jest.fn(),
	dataSourceNode: { id: 'node-1' },
	tryNextDataSourceNode: jest.fn().mockReturnValue(true),
	} as any
	mockP2P.askBinary.mockReturnValueOnce(null).mockReturnValueOnce({
	data: {
	wrappedAccounts: [{ accountId: 'account1', stateId: 'hash1', timestamp: 100, data: {} }],
	wrappedAccounts2: [],
	lastUpdateNeeded: true,
	highestTs: 100,
	delta: 0,
	},
	})
	// Act
	const result = await nodeSyncTracker.syncAccountData2('0x1', '0x2')
	// Assert
	expect(result).toBe(10)
	expect(mockP2P.askBinary).toHaveBeenCalledTimes(2)
	expect(nodeSyncTracker.dataSourceHelper.tryNextDataSourceNode).toHaveBeenCalledWith('syncAccountData2')
	expect(nodeSyncTracker.dataSourceHelper.tryNextDataSourceNode).toHaveBeenCalled() // Verify retry logic
	// Restore original method
	nodeSyncTracker.syncAccountData2 = originalMethod
	})