Skip to content

Conversation

@eablack
Copy link
Contributor

@eablack eablack commented Jan 8, 2026

This PR is part of a series migrating test files from @oclif/test v2 to v4 following the package upgrade in the foundation PR. This completes the final batch of remaining test files.

Summary

Migrates 16 test files covering spaces, ps, reviewapps, utilities, webhooks, and integration tests to be compatible with @oclif/test v4.1.15. This PR handles a diverse set of test scenarios including command tests, utility function tests, library tests, and integration tests.

Files Migrated (16 total)

Commands - ps (3 files):

  • test/unit/commands/ps/disable.unit.test.ts (2 tests)
  • test/unit/commands/ps/enable.unit.test.ts (11 tests)
  • test/unit/commands/ps/wait.unit.test.ts (7 tests)

Commands - reviewapps (2 files):

  • test/unit/commands/reviewapps/disable.unit.test.ts (10 tests)
  • test/unit/commands/reviewapps/enable.unit.test.ts (10 tests)

Commands - spaces (8 files):

  • test/unit/commands/spaces/drains/set.unit.test.ts (1 test)
  • test/unit/commands/spaces/peerings/accept.unit.test.ts (1 test)
  • test/unit/commands/spaces/ps.unit.test.ts (4 tests)
  • test/unit/commands/spaces/transfer.unit.test.ts (2 tests)
  • test/unit/commands/spaces/trusted-ips/add.unit.test.ts (3 tests)
  • test/unit/commands/spaces/trusted-ips/index.unit.test.ts (5 tests)
  • test/unit/commands/spaces/trusted-ips/remove.unit.test.ts (3 tests)

Library & Utils (3 files):

  • test/unit/lib/webhooks/base.unit.test.ts (3 tests)
  • test/unit/utils/keyValueParser.unit.test.ts (1 test)
  • test/unit/utils/paginator.unit.test.ts (2 tests)

Integration (1 file):

  • test/integration/run.integration.test.ts (3 tests passing, 3 skipped)

Total: 68 tests migrated ✅

Migration Changes

Before (v2) - Command tests:

test
  .nock('https://api.heroku.com:443', api => {
    api
      .get('/spaces/my-space/dynos')
      .reply(200, spaceDynos)
  })
  .stdout()
  .command(['spaces:ps', '--space', 'my-space'])
  .it('shows space dynos', ctx => {
    expect(ctx.stdout).to.equal('...')
  })

After (v4) - Command tests:

it('shows space dynos', async function () {
  nock('https://api.heroku.com:443')
    .get('/spaces/my-space/dynos')
    .reply(200, spaceDynos)
    .get('/spaces/my-space')
    .reply(200, {shield: false})

  const {stdout} = await runCommand(['spaces:ps', '--space', 'my-space'])

  expect(stdout).to.equal('...')
})

Before (v2) - Tests with stubs:

test
  .stub(hux, 'wait', () => Promise.resolve())
  .nock('https://api.heroku.com', api => {
    api.get(`/apps/${APP_NAME}/releases`).reply(200, [CURRENT])
  })
  .command(['ps:wait', '--app', APP_NAME])
  .it('waits for all dynos to be on latest release', ctx => {
    expect(ctx.stderr).to.contain('Waiting for every dyno')
  })

After (v4) - Tests with stubs:

it('waits for all dynos to be on latest release', async function () {
  sinon.stub(hux, 'wait').resolves()

  nock(API_HOST)
    .get(`/apps/${APP_NAME}/releases`)
    .reply(200, [CURRENT])
    .get(`/apps/${APP_NAME}/dynos`)
    .reply(200, [...])

  const {stderr} = await runCommand(['ps:wait', '--app', APP_NAME])

  expect(stderr).to.contain('Waiting for every dyno to be running v23... 2 / 2, done')
})

Before (v2) - Utility/library tests:

describe('webhooks type', function () {
  test
    .stdout()
    .do(function () {
      const webhookInfo = webhookObject.webhookType({pipeline: 'randomPipeline', app: ''})
      expect(webhookInfo).to.deep.equal({path: '/pipelines/randomPipeline', display: 'randomPipeline'})
    })
    .it('returns correct pipeline path and display info')
})

After (v4) - Utility/library tests:

describe('webhooks type', function () {
  it('returns correct pipeline path and display info', function () {
    const webhookInfo = webhookObject.webhookType({pipeline: 'randomPipeline', app: ''})
    expect(webhookInfo).to.deep.equal({path: '/pipelines/randomPipeline', display: 'randomPipeline'})
  })
})

Key Pattern Changes

  1. Standard runCommand(): Used @oclif/test v4's runCommand() for all command tests
  2. Async/await: Converted all tests to async/await functions
  3. Destructuring: Used {stdout, stderr, error} from runCommand results
  4. Error handling: Used {error} destructuring for error assertions instead of try/catch or .catch() chains
  5. Imports: Changed expect import from '@oclif/test' to 'chai'
  6. Stub handling: Moved sinon stubs inside test functions with sinon.stub() and proper sinon.restore() in afterEach
  7. Nock cleanup: Added afterEach(() => nock.cleanAll()) hooks for proper test isolation
  8. Helper functions: Created helper functions like dynoTestSetup() in ps/enable to reduce test duplication
  9. Integration tests: Converted complex chaining tests with custom setup functions to standard async/await patterns
  10. Utility tests: Migrated non-command tests (webhooks, keyValueParser, paginator) to standard mocha format without @oclif/test dependencies
  11. Uncommented tests: The lib/webhooks/base tests were previously commented out - uncommented and migrated them to v4
  12. Skip file cleanup: All .skip files properly deleted and tracked in git commits

Testing

  • ✅ All 68 migrated tests pass with npx mocha
  • ✅ 68 passing, 3 pending (skipped integration tests)
  • ✅ No test regressions
  • ✅ Proper test isolation with cleanup hooks

Notes

This PR completes the final batch of remaining test files for the @oclif/test v4 migration. The files covered a diverse range of testing scenarios:

  • Command tests: Standard pattern using runCommand() with nock for API mocking
  • Tests with stubs: Used sinon.stub() for mocking dependencies like hux.wait
  • Utility tests: Converted to standard mocha tests without @oclif/test chaining
  • Library tests: The webhooks/base tests were previously commented out and have been uncommented and migrated
  • Integration tests: Complex run command tests with process.argv manipulation and rendezvous connection mocking

Key learnings:

  • The ps/wait test required using hux.wait from @heroku/heroku-cli-util instead of ux.wait
  • Helper functions are valuable for reducing duplication in tests with similar setup (e.g., dynoTestSetup)
  • Integration tests benefit from beforeEach/afterEach hooks to save and restore process.argv

Related

@eablack eablack requested a review from a team as a code owner January 8, 2026 19:09
@eablack eablack mentioned this pull request Jan 8, 2026
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant