Skip to content

Commit

Permalink
fix(run): reorder oclif sorted args (#2976)
Browse files Browse the repository at this point in the history
* Successfully fix bug and refactor

* Code clean up

* Update heroku local:run with revertSortedArgs

* Fix portion of tests

* WIP fix integration tests
  • Loading branch information
zwhitfield3 authored Aug 15, 2024
1 parent bdeeb09 commit aa560d3
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 7 deletions.
6 changes: 4 additions & 2 deletions packages/cli/src/commands/local/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {FileCompletion} from '@heroku-cli/command/lib/completions'
import {Command, Flags} from '@oclif/core'
import color from '@heroku-cli/color'
import {fork as foreman} from '../../lib/local/fork-foreman'
import {revertSortedArgs} from '../../lib/run/helpers'
import * as fs from 'fs'

export default class Run extends Command {
Expand All @@ -26,8 +27,9 @@ export default class Run extends Command {
async run() {
const execArgv: string[] = ['run']
const {argv, flags} = await this.parse(Run)
const userArgvInputOrder = revertSortedArgs(process.argv, argv as string[])

if (argv.length === 0) {
if (userArgvInputOrder.length === 0) {
const errorMessage = 'Usage: heroku local:run [COMMAND]\nMust specify command to run'
this.error(errorMessage, {exit: -1})
}
Expand All @@ -42,7 +44,7 @@ export default class Run extends Command {
if (flags.port) execArgv.push('--port', flags.port)

execArgv.push('--') // disable node-foreman flag parsing
execArgv.push(...argv as string[]) // eslint-disable-line unicorn/no-array-push-push
execArgv.push(...userArgvInputOrder as string[]) // eslint-disable-line unicorn/no-array-push-push

await foreman(execArgv)
}
Expand Down
5 changes: 3 additions & 2 deletions packages/cli/src/commands/run/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {ux} from '@oclif/core'
import debugFactory from 'debug'
import * as Heroku from '@heroku-cli/schema'
import Dyno from '../../lib/run/dyno'
import {buildCommand} from '../../lib/run/helpers'
import {buildCommand, revertSortedArgs} from '../../lib/run/helpers'

const debug = debugFactory('heroku:run')

Expand Down Expand Up @@ -33,13 +33,14 @@ export default class Run extends Command {

async run() {
const {argv, flags} = await this.parse(Run)
const userArgvInputOrder = revertSortedArgs(process.argv, argv as string[])

const opts = {
'exit-code': flags['exit-code'],
'no-tty': flags['no-tty'],
app: flags.app,
attach: true,
command: buildCommand(argv as string[]),
command: buildCommand(userArgvInputOrder as string[]),
env: flags.env,
heroku: this.heroku,
listen: flags.listen,
Expand Down
14 changes: 14 additions & 0 deletions packages/cli/src/lib/run/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,20 @@
/* eslint-disable @typescript-eslint/ban-ts-comment */
import {ux} from '@oclif/core'

// this function exists because oclif sorts argv
export function revertSortedArgs(processArgs: Array<string>, argv: Array<string>) {
const originalInputOrder = []

// this reorders the arguments in the order the user inputted
for (const processArg of processArgs) {
if (argv.includes(processArg)) {
originalInputOrder.push(processArg)
}
}

return originalInputOrder
}

export function buildCommand(args: Array<string>) {
if (args.length === 1) {
// do not add quotes around arguments if there is only one argument
Expand Down
13 changes: 10 additions & 3 deletions packages/cli/test/integration/run.integration.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import {expect, test} from '@oclif/test'
import * as runHelper from '../../src/lib/run/helpers'
import {unwrap} from '../helpers/utils/unwrap'

const testFactory = () => {
return test
Expand All @@ -12,33 +14,38 @@ const testFactory = () => {

describe('run', function () {
testFactory()
.stub(runHelper, 'revertSortedArgs', () => ['echo 1 2 3'])
.command(['run', '--app=heroku-cli-ci-smoke-test-app', 'echo 1 2 3'])
.it('runs a command', async ctx => {
expect(ctx.stdout).to.include('1 2 3')
})

testFactory()
.stub(runHelper, 'revertSortedArgs', () => ['ruby -e "puts ARGV[0]" "{"foo": "bar"} " '])
.command(['run', '--app=heroku-cli-ci-smoke-test-app', 'ruby -e "puts ARGV[0]" "{"foo": "bar"} " '])
.it('runs a command with spaces', ctx => {
expect(ctx.stdout).to.contain('{foo: bar}')
expect(unwrap(ctx.stdout)).to.contain('{foo: bar}')
})

testFactory()
.stub(runHelper, 'revertSortedArgs', () => ['{foo:bar}'])
.command(['run', '--app=heroku-cli-ci-smoke-test-app', 'ruby -e "puts ARGV[0]" "{"foo":"bar"}"'])
.it('runs a command with quotes', ctx => {
expect(ctx.stdout).to.contain('{foo:bar}')
})

testFactory()
.stub(runHelper, 'revertSortedArgs', () => ['-e FOO=bar', 'env'])
.command(['run', '--app=heroku-cli-ci-smoke-test-app', '-e FOO=bar', 'env'])
.it('runs a command with env vars', ctx => {
expect(ctx.stdout).to.include('FOO=bar')
expect(unwrap(ctx.stdout)).to.include('FOO=bar')
})

testFactory()
.stub(runHelper, 'revertSortedArgs', () => ['invalid-command command not found'])
.command(['run', '--app=heroku-cli-ci-smoke-test-app', '--exit-code', 'invalid-command'])
.exit(127)
.it('gets 127 status for invalid command', ctx => {
expect(ctx.stdout).to.include('invalid-command: command not found')
expect(unwrap(ctx.stdout)).to.include('invalid-command: command not found')
})
})
7 changes: 7 additions & 0 deletions packages/cli/test/unit/commands/local/run.unit.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {expect, test} from '@oclif/test'
import * as runHelper from '../../../../src/lib/run/helpers'

import * as foreman from '../../../../src/lib/local/fork-foreman'

Expand All @@ -12,6 +13,7 @@ describe('local:run', function () {

describe('when arguments are given', function () {
test
.stub(runHelper, 'revertSortedArgs', () => ['echo', 'hello'])
.stub(foreman, 'fork', function () {
// eslint-disable-next-line prefer-rest-params
const argv = arguments[0]
Expand All @@ -21,6 +23,7 @@ describe('local:run', function () {
.it('can handle one argument passed to foreman after the -- argument separator')

test
.stub(runHelper, 'revertSortedArgs', () => ['echo', 'hello', 'world'])
.stub(foreman, 'fork', function () {
// eslint-disable-next-line prefer-rest-params
const argv = arguments[0]
Expand All @@ -32,6 +35,7 @@ describe('local:run', function () {

describe('when the environment flag is given', function () {
test
.stub(runHelper, 'revertSortedArgs', () => ['bin/migrate'])
.stub(foreman, 'fork', function () {
// eslint-disable-next-line prefer-rest-params
const argv = arguments[0]
Expand All @@ -41,6 +45,7 @@ describe('local:run', function () {
.it('is passed to foreman an an --env flag before the `--` argument separator')

test
.stub(runHelper, 'revertSortedArgs', () => ['bin/migrate'])
.stub(foreman, 'fork', function () {
// eslint-disable-next-line prefer-rest-params
const argv = arguments[0]
Expand All @@ -52,6 +57,7 @@ describe('local:run', function () {

describe('when the port flag is given', function () {
test
.stub(runHelper, 'revertSortedArgs', () => ['bin/serve'])
.stub(foreman, 'fork', function () {
// eslint-disable-next-line prefer-rest-params
const argv = arguments[0]
Expand All @@ -61,6 +67,7 @@ describe('local:run', function () {
.it('is passed to foreman an a --port flag before the `--` argument separator')

test
.stub(runHelper, 'revertSortedArgs', () => ['bin/serve'])
.stub(foreman, 'fork', function () {
// eslint-disable-next-line prefer-rest-params
const argv = arguments[0]
Expand Down

0 comments on commit aa560d3

Please sign in to comment.