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 PR 8 of 11 in the incremental migration plan.

Description

Migrates 15 test files in the Apps Core category to be compatible with @oclif/test v4.1.15, including tests for app lifecycle management (create, destroy, info, rename, open), app organization (favorites, errors), stack management, rake command, and confirmation dialogs.

Files Migrated (15 total)

  • test/unit/commands/apps/create.unit.test.ts (11 tests)
  • test/unit/commands/apps/destroy.unit.test.ts (3 tests)
  • test/unit/commands/apps/errors.unit.test.ts (5 tests)
  • test/unit/commands/apps/favorites/add.unit.test.ts (3 tests)
  • test/unit/commands/apps/favorites/index.unit.test.ts (2 tests)
  • test/unit/commands/apps/favorites/remove.unit.test.ts (2 tests)
  • test/unit/commands/apps/index.unit.test.ts (16 tests)
  • test/unit/commands/apps/info.unit.test.ts (12 tests)
  • test/unit/commands/apps/leave.unit.test.ts (3 tests)
  • test/unit/commands/apps/open.unit.test.ts (2 tests)
  • test/unit/commands/apps/rename.unit.test.ts (2 tests)
  • test/unit/commands/apps/stacks/index.unit.test.ts (2 tests)
  • test/unit/commands/apps/stacks/set.unit.test.ts (3 tests)
  • test/unit/commands/rake.unit.test.ts (3 tests)
  • test/unit/lib/confirm-command.unit.test.ts (5 tests)

Total: 74 tests migrated ✅

Migration Changes

All tests were updated to use the new @oclif/test v4 API:

Before (v2):

test
  .stdout()
  .stderr()
  .nock('https://api.heroku.com:443', api => {
    api.get('/apps/myapp')
      .reply(200, app)
    api.get('/apps/myapp/addons')
      .reply(200, addons)
  })
  .command(['apps:info', '-a', 'myapp'])
  .it('shows app info', ({stdout, stderr}) => {
    expect(stdout).to.equal(BASE_INFO)
    expect(stderr).to.equal('')
  })

After (v4):

it('shows app info', async function () {
  nock('https://api.heroku.com:443')
    .get('/apps/myapp')
    .reply(200, app)
    .get('/apps/myapp/addons')
    .reply(200, addons)

  const {stdout, stderr} = await runCommand(['apps:info', '-a', 'myapp'])

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

Key Pattern Changes

  • API: Replaced chaining test API with runCommand() function
  • Async: Converted callback-style tests to async/await
  • Context: Destructured {stdout, stderr, error} from result instead of ctx parameter
  • Cleanup: Added afterEach(() => nock.cleanAll()) hooks for nock tests
  • Nock Setup: Moved nock setup inside each test for better isolation
  • Error Handling: Use error object from return value for expected errors instead of .catch()
  • Custom Helper: Used runCommandHelper for tests with sinon stubs (create with manifest, open, rake, confirm-command)

Special Cases

Tests with Sinon Stubs

Some tests required the custom runCommandHelper instead of @oclif/test v4's runCommand because they use sinon stubs:

// apps/create - manifest flag test with readManifest stub
const readManifestStub = sinon.stub(ManifestModule, 'readManifest')
await runCommandHelper(CreateCommand, ['--app', appName, '--manifest'])

// apps/open - childProcess.spawn stub
const spawnStub = sinon.stub(childProcess, 'spawn')
await runCommandHelper(OpenCommand, ['-a', 'myapp'])

// rake - Dyno.prototype.start stub
sinon.stub(Dyno.prototype, 'start')
await runCommandHelper(RakeCommand, ['--app=...', 'test'])

// lib/confirm-command - hux.prompt stub
sinon.stub(hux, 'prompt').resolves('app')
await new ConfirmCommand().confirm('app')

Addon Parsing Issue

The apps/create addon tests initially failed with @oclif/test v4's runCommand - comma-separated addons weren't parsed correctly. Using runCommandHelper resolved this issue.

Error Handling Pattern

For tests expecting errors:

Before (v2):

test
  .command(['apps:destroy', '--app', 'myapp'])
  .catch(error => expect(error.message).to.contain('app name required'))
  .it('errors without confirmation')

After (v4):

it('errors without confirmation', async function () {
  const {error} = await runCommand(['apps:destroy', '--app', 'myapp'])
  
  expect(error?.message).to.contain('app name required')
})

Testing

✅ All 74 migrated tests pass locally
✅ No regressions in other test suites
✅ Build passes

Test Results:

  • apps/create: 11 passing
  • apps/destroy: 3 passing
  • apps/errors: 5 passing
  • apps/favorites: 7 passing (3 files)
  • apps/index: 16 passing
  • apps/info: 12 passing
  • apps/leave: 3 passing
  • apps/open: 2 passing
  • apps/rename: 2 passing
  • apps/stacks: 5 passing (2 files)
  • rake: 3 passing
  • lib/confirm-command: 5 passing

Related

@eablack eablack requested a review from a team as a code owner January 8, 2026 00:20
@eablack eablack mentioned this pull request Jan 8, 2026
12 tasks
Copy link
Contributor

@zwhitfield3 zwhitfield3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me and CI is passing!

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.

2 participants