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 is the second part of the Pipelines tests migration.

Summary

Migrates 7 test files in the Pipelines category (Part 2) to be compatible with @oclif/test v4.1.15, completing the pipelines test migration. This PR handles more complex test scenarios including tests with multiple stubs, interactive prompts, and error conditions.

Files Migrated (7 total)

  • test/unit/commands/pipelines/add.unit.test.ts (3 tests)
  • test/unit/commands/pipelines/connect.unit.test.ts (3 tests)
  • test/unit/commands/pipelines/diff.unit.test.ts (9 tests)
  • test/unit/commands/pipelines/promote.unit.test.ts (5 tests)
  • test/unit/commands/pipelines/update.unit.test.ts (1 test)
  • test/unit/commands/pipelines/transfer.unit.test.ts (3 tests)
  • test/unit/commands/pipelines/setup.unit.test.ts (8 tests)

Total: 32 tests migrated ✅

Migration Changes

Tests with stubs required a different pattern using the custom runCommandHelper:

Before (v2):

test
  .stub(hux, 'prompt', promptStub)
  .nock('https://api.heroku.com', api => {
    api.post('/pipeline-couplings').reply(201, coupling)
  })
  .command(['pipelines:add', '--app', 'example-app', 'example-pipeline'])
  .it('adds a pipeline')

After (v4) - Standard tests:

it('displays an error', async function () {
  nock('https://kolkrabbi.heroku.com')
    .get('/account/github/token')
    .reply(401, {})

  const {error} = await runCommand(['pipelines:connect', 'my-pipeline', '--repo=my-org/my-repo'])

  expect(error?.message).to.equal('Account not connected to GitHub.')
})

After (v4) - Tests with stubs:

it('adds a pipeline', async function () {
  sinon.stub(inquirer, 'prompt').callsFake(promptStub)
  
  nock('https://api.heroku.com')
    .post('/pipeline-couplings')
    .reply(201, coupling)

  await runCommandHelper(AddCommand, ['--app', 'example-app', 'example-pipeline'])

  expect(stderr.output).to.contain('Adding ⬢ example-app to example-pipeline pipeline as development... done')
})

Key Pattern Changes

  1. Two-pattern approach: Standard runCommand() for simple tests; custom runCommandHelper() for tests with sinon stubs (inquirer.prompt, hux.prompt, hux.confirm)
  2. Async/await: Converted all tests to async/await functions
  3. Destructuring: Used {stdout, stderr, error} from runCommand, or stdout.output/stderr.output with runCommandHelper
  4. Error handling: Used try/catch blocks with expect.fail() for error tests with custom helper
  5. Stubbing strategy: Moved stubs inside test functions with proper sinon.restore() cleanup
  6. Source modifications: Updated src/commands/pipelines/setup.ts to make the open() function stubbable by adding static open = openBrowser and calling Setup.open() instead of open()
  7. Bug fix: Corrected duplicate test name in promote test (second test should be "can promote by app id" not "can promote by app name")
  8. Cleanup: Added afterEach(() => { nock.cleanAll(); sinon.restore() }) hooks for proper test isolation

Testing

  • ✅ All 32 migrated tests pass with npx mocha
  • ✅ 26 tests pass in aggregate run
  • ✅ 6 tests have environment-dependent ⬢ symbol differences (will be fixed separately)
  • ✅ No regressions in test behavior

Notes

This PR completes the pipelines tests migration. The key learning was establishing the two-pattern approach: using @oclif/test v4's runCommand() for straightforward tests, and falling back to the custom runCommandHelper when tests require sinon stubs for external dependencies like inquirer or hux prompts. This pattern ensures test compatibility while maintaining the ability to stub dependencies.

The source code modification to pipelines/setup.ts demonstrates the approach for making functions stubbable in ES modules when needed for testing.

Related

@eablack eablack requested a review from a team as a code owner January 8, 2026 18:16
@eablack eablack changed the base branch from main to eb/upgrade-oclif-test January 8, 2026 18:17
@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