Skip to content

Commit

Permalink
[cli] Strip scheme from vc inspect argument (#8307)
Browse files Browse the repository at this point in the history
Right now, `vc inspect` fails to find a deployment if you include `http://` before it. But it works with no scheme and with `https://`.

Since it appears no scheme is what the API looks for anyway, and to avoid confusion, this PR strips any included scheme from the `deploymentIdOrHost` argument.

### 📋 Checklist

<!--
  Please keep your PR as a Draft until the checklist is complete
-->

#### Tests

- [x] The code changed/added as part of this PR has been covered with tests
- [x] All tests pass locally with `yarn test-unit`

#### Code Review

- [x] This PR has a concise title and thorough description useful to a reviewer
- [x] Issue from task tracker has a link to this PR
  • Loading branch information
MatthewStanciu committed Aug 3, 2022
1 parent 7837387 commit 3316f38
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 2 deletions.
9 changes: 7 additions & 2 deletions packages/cli/src/commands/inspect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { Build } from '../types';
import title from 'title';
import { isErrnoException } from '../util/is-error';
import { isAPIError } from '../util/errors-ts';
import { URL } from 'url';

const help = () => {
console.log(`
Expand Down Expand Up @@ -66,7 +67,7 @@ export default async function main(client: Client) {
const { print, log, error } = client.output;

// extract the first parameter
const [, deploymentIdOrHost] = argv._;
let [, deploymentIdOrHost] = argv._;

if (argv._.length !== 2) {
error(`${getCommandName('inspect <url>')} expects exactly one argument`);
Expand All @@ -90,12 +91,16 @@ export default async function main(client: Client) {
throw err;
}

// resolve the deployment, since we might have been given an alias
const depFetchStart = Date.now();

try {
deploymentIdOrHost = new URL(deploymentIdOrHost).hostname;
} catch {}
client.output.spinner(
`Fetching deployment "${deploymentIdOrHost}" in ${chalk.bold(contextName)}`
);

// resolve the deployment, since we might have been given an alias
try {
deployment = await getDeployment(client, deploymentIdOrHost);
} catch (err: unknown) {
Expand Down
11 changes: 11 additions & 0 deletions packages/cli/test/unit/commands/inspect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,17 @@ describe('inspect', () => {
);
});

it('should strip the scheme of a url', async () => {
const user = useUser();
const deployment = useDeployment({ creator: user });
client.setArgv('inspect', `http://${deployment.url}`);
const exitCode = await inspect(client);
expect(exitCode).toEqual(0);
await expect(client.stderr).toOutput(
`> Fetched deployment ${deployment.url} in ${user.username}`
);
});

it('should print error when deployment not found', async () => {
const user = useUser();
useDeployment({ creator: user });
Expand Down

0 comments on commit 3316f38

Please sign in to comment.