Skip to content

Add network class tests #512

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
BelfordZ wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Add network class tests #512

BelfordZ wants to merge 1 commit into from

Conversation

@BelfordZ
Copy link
Contributor

@BelfordZ BelfordZ commented May 22, 2025 •
edited by github-actions bot
Loading

User description

Summary

  • add unit tests for NetworkClass covering setup, tell, ask and shutdown
  • mock sockets and other dependencies

Testing

  • npm test

PR Type

Tests

Description

  • Add comprehensive unit tests for NetworkClass methods

  • Mock dependencies including logger, profiler, and NodeList

  • Test setup, tell, ask, and shutdown behaviors

  • Validate error handling and edge cases in methods

Changes walkthrough 📝

Relevant files
Tests
networkClass.test.ts
Add unit tests for NetworkClass methods and behaviors       

test/unit/src/network/networkClass.test.ts

  • Introduces unit tests for NetworkClass covering setup, tell, ask, and
    shutdown
  • Mocks external dependencies such as logger, profiler, and NodeList
  • Tests normal and error scenarios, including missing fields and
    timeouts
  • Ensures correct method calls and state changes within NetworkClass
    		• +160/-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
    Copy link

    github-actions bot commented May 22, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🏅 Score: 95
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Mock Cleanup

    Ensure that all mocks, especially those created with jest.spyOn or jest.fn, are properly restored or cleared after each test to avoid cross-test contamination and maintain test isolation.

     
    beforeEach(() => {
  network = new NetworkClass(baseConfig, loggerMock as any)
})

describe('NetworkClass.setup', () => {
  test('calls internal and external setup with valid ipInfo', async () => {
    const ipInfo = {
      externalIp: '1.1.1.1',
      externalPort: 1111,
      internalIp: '2.2.2.2',
      internalPort: 2222,
    }
    const internalSpy = jest
      .spyOn(network as any, '_setupInternal')
      .mockImplementation(() => undefined)
    const externalSpy = jest
      .spyOn(network as any, '_setupExternal')
      .mockResolvedValue('ioMock')

    const result = await network.setup(ipInfo, 'secret')

    expect(internalSpy).toHaveBeenCalled()
    expect(externalSpy).toHaveBeenCalled()
    expect(network.ipInfo).toBe(ipInfo)
    expect(result).toBe('ioMock')
    expect(loggerMock.setPlaybackIPInfo).toHaveBeenCalledWith(ipInfo)
  })

  test('throws error when required ipInfo fields missing', async () => {
    await expect(network.setup({} as any, 'secret')).rejects.toThrow('externalIp')
  })
})

describe('NetworkClass.tell', () => {
  test('sends message to each node', async () => {
    const snMock = { send: jest.fn().mockImplementation(() => Promise.resolve()) }
    ;(network as any).sn = snMock
    const nodes = [
      { internalIp: '127.0.0.1', internalPort: 10 },
      { internalIp: '127.0.0.2', internalPort: 11 },
    ] as any

    await network.tell(nodes, 'route', { tracker: 't1' })

    expect(snMock.send).toHaveBeenCalledTimes(2)
    expect(snMock.send).toHaveBeenCalledWith(10, '127.0.0.1', {
      route: 'route',
      payload: { tracker: 't1' },
    })
  })

  test('does nothing when node list is empty', async () => {
    const snMock = { send: jest.fn().mockImplementation(() => Promise.resolve()) }
    ;(network as any).sn = snMock

    await network.tell([], 'route', {})

    expect(snMock.send).not.toHaveBeenCalled()
  })
})

describe('NetworkClass.ask', () => {
  test('resolves with response on success', async () => {
    const snMock = {
      send: jest.fn((p, ip, data, timeout, onRes: any) => {
        onRes('resp')
        return Promise.resolve()
      }),
    }
    ;(network as any).sn = snMock
    const node = { internalIp: '127.0.0.1', internalPort: 10 } as any

    await expect(network.ask(node, 'route', { tracker: 't' })).resolves.toBe('resp')
    expect(snMock.send).toHaveBeenCalled()
  })

  test('rejects on timeout', async () => {
    const snMock = {
      send: jest.fn((p, ip, data, timeout, onRes: any, onTimeout: any) => {
        onTimeout()
        return Promise.resolve()
      }),
    }
    ;(network as any).sn = snMock
    const node = { internalIp: '127.0.0.1', internalPort: 10 } as any

    await expect(network.ask(node, 'route', { tracker: 't' })).rejects.toThrow('Request timed out')
    expect(snMock.send).toHaveBeenCalled()
  })
})

describe('NetworkClass.shutdown', () => {
  test('closes external server if present', async () => {
    const close = jest.fn()
    const unref = jest.fn()
    ;(network as any).extServer = { close, unref }

    await network.shutdown()

    expect(close).toHaveBeenCalled()
    expect(unref).toHaveBeenCalled()
  })
})
    Linear Issue Linkage

    Confirm that the PR is linked to a Linear issue as required by the review guidelines, since there is no explicit reference to a Linear issue key in the test file or PR metadata.

     
    import { describe, beforeEach, test, expect, jest } from '@jest/globals'

jest.mock('../../../../src/utils/profiler', () => ({
  profilerInstance: {
    profileSectionStart: jest.fn(),
    profileSectionEnd: jest.fn(),
    scopedProfileSectionStart: jest.fn(),
    scopedProfileSectionEnd: jest.fn(),
  },
}))

jest.mock('../../../../src/p2p/NodeList', () => ({
  nodes: new Map(),
  byPubKey: new Map(),
  activeByIdOrder: [],
  byIdOrder: [],
  reset: jest.fn(),
}))

jest.mock('../../../../src/logger', () => ({
  __esModule: true,
  default: jest.fn().mockImplementation(() => ({
    getLogger: jest.fn().mockReturnValue({ info: jest.fn(), debug: jest.fn(), error: jest.fn() }),
    setPlaybackIPInfo: jest.fn(),
    playbackLog: jest.fn(),
  })),
  logFlags: { verbose: false, error: false, debug: false, info: false, net_verbose: false, playback: false },
}))

import { NetworkClass } from '../../../../src/network'

// Minimal logger mock used by NetworkClass
const loggerMock = {
  getLogger: jest.fn().mockReturnValue({
    info: jest.fn(),
    debug: jest.fn(),
    error: jest.fn(),
  }),
  setPlaybackIPInfo: jest.fn(),
  playbackLog: jest.fn(),
}

// Basic configuration object with only fields required by NetworkClass
const baseConfig: any = {
  network: { timeout: 5 },
  p2p: {
    useLruCacheForSocketMgmt: false,
    lruCacheSizeForSocketMgmt: 1000,
    payloadSizeLimitInBytes: 1024,
    headerSizeLimitInBytes: 512,
  },
  crypto: { hashKey: 'hash' },
  debug: {},
}

let network: NetworkClass

beforeEach(() => {
  network = new NetworkClass(baseConfig, loggerMock as any)
})

describe('NetworkClass.setup', () => {
  test('calls internal and external setup with valid ipInfo', async () => {
    const ipInfo = {
      externalIp: '1.1.1.1',
      externalPort: 1111,
      internalIp: '2.2.2.2',
      internalPort: 2222,
    }
    const internalSpy = jest
      .spyOn(network as any, '_setupInternal')
      .mockImplementation(() => undefined)
    const externalSpy = jest
      .spyOn(network as any, '_setupExternal')
      .mockResolvedValue('ioMock')

    const result = await network.setup(ipInfo, 'secret')

    expect(internalSpy).toHaveBeenCalled()
    expect(externalSpy).toHaveBeenCalled()
    expect(network.ipInfo).toBe(ipInfo)
    expect(result).toBe('ioMock')
    expect(loggerMock.setPlaybackIPInfo).toHaveBeenCalledWith(ipInfo)
  })

  test('throws error when required ipInfo fields missing', async () => {
    await expect(network.setup({} as any, 'secret')).rejects.toThrow('externalIp')
  })
})

describe('NetworkClass.tell', () => {
  test('sends message to each node', async () => {
    const snMock = { send: jest.fn().mockImplementation(() => Promise.resolve()) }
    ;(network as any).sn = snMock
    const nodes = [
      { internalIp: '127.0.0.1', internalPort: 10 },
      { internalIp: '127.0.0.2', internalPort: 11 },
    ] as any

    await network.tell(nodes, 'route', { tracker: 't1' })

    expect(snMock.send).toHaveBeenCalledTimes(2)
    expect(snMock.send).toHaveBeenCalledWith(10, '127.0.0.1', {
      route: 'route',
      payload: { tracker: 't1' },
    })
  })

  test('does nothing when node list is empty', async () => {
    const snMock = { send: jest.fn().mockImplementation(() => Promise.resolve()) }
    ;(network as any).sn = snMock

    await network.tell([], 'route', {})

    expect(snMock.send).not.toHaveBeenCalled()
  })
})

describe('NetworkClass.ask', () => {
  test('resolves with response on success', async () => {
    const snMock = {
      send: jest.fn((p, ip, data, timeout, onRes: any) => {
        onRes('resp')
        return Promise.resolve()
      }),
    }
    ;(network as any).sn = snMock
    const node = { internalIp: '127.0.0.1', internalPort: 10 } as any

    await expect(network.ask(node, 'route', { tracker: 't' })).resolves.toBe('resp')
    expect(snMock.send).toHaveBeenCalled()
  })

  test('rejects on timeout', async () => {
    const snMock = {
      send: jest.fn((p, ip, data, timeout, onRes: any, onTimeout: any) => {
        onTimeout()
        return Promise.resolve()
      }),
    }
    ;(network as any).sn = snMock
    const node = { internalIp: '127.0.0.1', internalPort: 10 } as any

    await expect(network.ask(node, 'route', { tracker: 't' })).rejects.toThrow('Request timed out')
    expect(snMock.send).toHaveBeenCalled()
  })
})

describe('NetworkClass.shutdown', () => {
  test('closes external server if present', async () => {
    const close = jest.fn()
    const unref = jest.fn()
    ;(network as any).extServer = { close, unref }

    await network.shutdown()

    expect(close).toHaveBeenCalled()
    expect(unref).toHaveBeenCalled()
  })
})

    @github-actions
    Copy link

    github-actions bot commented May 22, 2025

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    No reviews

    Assignees

    No one assigned

    Labels

    codex Review effort 2/5

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    2 participants

    @BelfordZ