Skip to content

Commit

Permalink
Fix broken install command (#470)
Browse files Browse the repository at this point in the history
Regression from #465.

Before:

```
❯ ../bin/elm-test install truqu/elm-md5
I am having trouble with this argument:

    t

It is supposed to be a <package> value, like one of these:

    elm/html
    elm/http
    elm/svg
    elm/time
```

After:

```
❯ ../bin/elm-test install truqu/elm-md5
Here is my plan:

  Add:
    truqu/elm-md5            1.1.0
    zwilias/elm-utf-tools    2.0.1

Would you like me to update your elm.json accordingly? [Y/n]:
Success!
```

This happened because I thought the first argument of the `.action` callback is always an array. Wrong! The signature depends on the `.command` syntax provided:

```js
program
  .command('name <one> <two> [three] [rest...]')
  .action((one: string, two: string, three: string | undefined, rest: Array<string>, cmd: Command) => {
    // Do stuff
  });
```

For the install command we have:

```js
program
  .command('install <package>')
  .action((packageName: string, cmd: Command) => {})
```

Before we tried to destructure the first argument (thinking it was an array), so we only got the first letter of the package name.

Unfortunately our test suite doesn’t catch this.
  • Loading branch information
lydell authored Nov 17, 2020
1 parent 85d370a commit dbc2d18
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/elm-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ function main() {
.description(
'Like `elm install package`, except it installs to "test-dependencies" in your elm.json'
)
.action(([packageName], cmd) => {
.action((packageName, cmd) => {
if (cmd.args.length > 1) {
// Unfortunately commander is very permissive about extra arguments. Therefore,
// we manually check for excessive arguments.
Expand Down
6 changes: 6 additions & 0 deletions tests/flags.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,12 @@ describe('flags', () => {
assert(!runResult.stdout.includes('FINDME'));
assert(!runResult.stderr.includes('FINDMETWICE'));
}).timeout(60000);

it('should exit with success if package already installed', () => {
const runResult = execElmTest(['install', 'elm-explorations/test']);
console.log(runResult);
assert.strictEqual(runResult.status, 0);
}).timeout(60000);
});

describe('elm-test make', () => {
Expand Down

0 comments on commit dbc2d18

Please sign in to comment.