From 4c436ce2ba35ca190adbd998c303f6ba78df455a Mon Sep 17 00:00:00 2001 From: Zane Whitfield Date: Thu, 19 Sep 2024 16:53:18 -0700 Subject: [PATCH 01/19] WIP add command structure --- src/commands/ai/models/destroy.ts | 35 +++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 src/commands/ai/models/destroy.ts diff --git a/src/commands/ai/models/destroy.ts b/src/commands/ai/models/destroy.ts new file mode 100644 index 0000000..1469a69 --- /dev/null +++ b/src/commands/ai/models/destroy.ts @@ -0,0 +1,35 @@ +import {Args, ux} from '@oclif/core' +import {flags} from '@heroku-cli/command' +import {ModelList} from '../../../lib/ai/types' +import Command from '../../../lib/base' + +export default class Destroy extends Command { + static description = 'destroy an existing AI model resource' + + static flags = { + app: flags.app({required: true, description: 'app to run command against'}), + confirm: flags.string({char: 'c'}), + force: flags.boolean({char: 'f', description: 'allow destruction even if connected to other apps'}), + remote: flags.remote({description: 'git remote of app to use'}), + } + + static args = { + modelResource: Args.string({required: true, description: 'The resource ID or alias of the model resource to destroy.'}), + }; + + static examples = [ + '$ heroku ai:models:destroy claude-3-5-sonnet-acute-43973', + ] + + public async run(): Promise { + const {flags, argv} = await this.parse(Destroy) + const {app, confirm} = flags + const force = flags.force || process.env.HEROKU_FORCE === '1' + await this.configureHerokuAIClient() + + const herokuAIClient = this.herokuAI + const urlPath = '/available-models' + + const {body: availableModels} = await herokuAIClient.get(urlPath) + } +} From 6b0203486798899794b7e81964e4a2a0da6f0c00 Mon Sep 17 00:00:00 2001 From: Zane Whitfield Date: Tue, 24 Sep 2024 08:46:13 -0700 Subject: [PATCH 02/19] WIP return ai model addon ID --- src/commands/ai/models/destroy.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/commands/ai/models/destroy.ts b/src/commands/ai/models/destroy.ts index 1469a69..85a47b1 100644 --- a/src/commands/ai/models/destroy.ts +++ b/src/commands/ai/models/destroy.ts @@ -22,14 +22,18 @@ export default class Destroy extends Command { ] public async run(): Promise { - const {flags, argv} = await this.parse(Destroy) + const {flags, args} = await this.parse(Destroy) const {app, confirm} = flags + const {modelResource} = args const force = flags.force || process.env.HEROKU_FORCE === '1' - await this.configureHerokuAIClient() - const herokuAIClient = this.herokuAI - const urlPath = '/available-models' + await this.configureHerokuAIClient(modelResource, app) - const {body: availableModels} = await herokuAIClient.get(urlPath) + const aiAddonId = this.addon.id + console.log('aiAddonId', aiAddonId) + + // const herokuClient = this.herokuAI + + // const {body: availableModels} = await herokuAIClient.get(urlPath) } } From 4c4949507ff525792e18ece03af6ab071ddbef63 Mon Sep 17 00:00:00 2001 From: Zane Whitfield Date: Tue, 24 Sep 2024 09:14:25 -0700 Subject: [PATCH 03/19] WIP add destroy_addon util --- src/lib/ai/models/destroy_addon.ts | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 src/lib/ai/models/destroy_addon.ts diff --git a/src/lib/ai/models/destroy_addon.ts b/src/lib/ai/models/destroy_addon.ts new file mode 100644 index 0000000..ddfeb16 --- /dev/null +++ b/src/lib/ai/models/destroy_addon.ts @@ -0,0 +1,21 @@ +import color from '@heroku-cli/color' +import {ux} from '@oclif/core' +import {APIClient} from '@heroku-cli/command' +import * as Heroku from '@heroku-cli/schema' + +export default async function (heroku: APIClient, addon: Heroku.AddOn, force = false) { + const addonName = addon.name || '' + + ux.action.start(`Destroying ${color.addon(addonName)} in the background.\n The app will restart when complete...`) + + const {body: addonDelete} = await heroku.delete(`/apps/${addon.app?.id}/addons/${addon.id}`, { + headers: {'Accept-Expansion': 'plan'}, + body: {force}, + }).catch(error => { + const error_ = error.body && error.body.message ? new Error(`The add-on was unable to be destroyed: ${error.body.message}.`) : new Error(`The add-on was unable to be destroyed: ${error}.`) + throw error_ + }) + + ux.action.stop() + return addonDelete +} From db54dbcd7c250674de29aac7af5aa6fd7573adc9 Mon Sep 17 00:00:00 2001 From: Zane Whitfield Date: Tue, 24 Sep 2024 09:33:45 -0700 Subject: [PATCH 04/19] WIP update destroy_addon util --- src/commands/ai/models/destroy.ts | 7 ++++--- src/lib/ai/models/destroy_addon.ts | 5 +++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/commands/ai/models/destroy.ts b/src/commands/ai/models/destroy.ts index 85a47b1..1b24432 100644 --- a/src/commands/ai/models/destroy.ts +++ b/src/commands/ai/models/destroy.ts @@ -1,5 +1,6 @@ import {Args, ux} from '@oclif/core' import {flags} from '@heroku-cli/command' +import destroyAddon from '../../../lib/ai/models/destroy_addon' import {ModelList} from '../../../lib/ai/types' import Command from '../../../lib/base' @@ -29,10 +30,10 @@ export default class Destroy extends Command { await this.configureHerokuAIClient(modelResource, app) - const aiAddonId = this.addon.id - console.log('aiAddonId', aiAddonId) + const aiAddon = this.addon - // const herokuClient = this.herokuAI + const destroyAddonResponse = await destroyAddon(this.config, aiAddon, force) + console.log('destroyAddonResponse', destroyAddonResponse) // const {body: availableModels} = await herokuAIClient.get(urlPath) } diff --git a/src/lib/ai/models/destroy_addon.ts b/src/lib/ai/models/destroy_addon.ts index ddfeb16..be19df8 100644 --- a/src/lib/ai/models/destroy_addon.ts +++ b/src/lib/ai/models/destroy_addon.ts @@ -3,12 +3,13 @@ import {ux} from '@oclif/core' import {APIClient} from '@heroku-cli/command' import * as Heroku from '@heroku-cli/schema' -export default async function (heroku: APIClient, addon: Heroku.AddOn, force = false) { +export default async function (config: any, addon: Heroku.AddOn, force = false) { const addonName = addon.name || '' + const herokuClient = new APIClient(config) ux.action.start(`Destroying ${color.addon(addonName)} in the background.\n The app will restart when complete...`) - const {body: addonDelete} = await heroku.delete(`/apps/${addon.app?.id}/addons/${addon.id}`, { + const {body: addonDelete} = await herokuClient.delete(`/apps/${addon.app?.id}/addons/${addon.id}`, { headers: {'Accept-Expansion': 'plan'}, body: {force}, }).catch(error => { From eb2d37c26021e8a4e601dcb96d88f9dbbb3cbc92 Mon Sep 17 00:00:00 2001 From: Zane Whitfield Date: Tue, 24 Sep 2024 19:01:25 -0700 Subject: [PATCH 05/19] Clean up destroy_addon util --- src/commands/ai/models/destroy.ts | 8 ++------ src/lib/ai/models/destroy_addon.ts | 8 ++++---- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/commands/ai/models/destroy.ts b/src/commands/ai/models/destroy.ts index 1b24432..b063fba 100644 --- a/src/commands/ai/models/destroy.ts +++ b/src/commands/ai/models/destroy.ts @@ -1,7 +1,6 @@ import {Args, ux} from '@oclif/core' import {flags} from '@heroku-cli/command' import destroyAddon from '../../../lib/ai/models/destroy_addon' -import {ModelList} from '../../../lib/ai/types' import Command from '../../../lib/base' export default class Destroy extends Command { @@ -16,7 +15,7 @@ export default class Destroy extends Command { static args = { modelResource: Args.string({required: true, description: 'The resource ID or alias of the model resource to destroy.'}), - }; + } static examples = [ '$ heroku ai:models:destroy claude-3-5-sonnet-acute-43973', @@ -32,9 +31,6 @@ export default class Destroy extends Command { const aiAddon = this.addon - const destroyAddonResponse = await destroyAddon(this.config, aiAddon, force) - console.log('destroyAddonResponse', destroyAddonResponse) - - // const {body: availableModels} = await herokuAIClient.get(urlPath) + await destroyAddon(this.config, aiAddon, force) } } diff --git a/src/lib/ai/models/destroy_addon.ts b/src/lib/ai/models/destroy_addon.ts index be19df8..39d5465 100644 --- a/src/lib/ai/models/destroy_addon.ts +++ b/src/lib/ai/models/destroy_addon.ts @@ -1,15 +1,16 @@ import color from '@heroku-cli/color' import {ux} from '@oclif/core' +import {Config} from '@oclif/core' import {APIClient} from '@heroku-cli/command' import * as Heroku from '@heroku-cli/schema' -export default async function (config: any, addon: Heroku.AddOn, force = false) { +export default async function (config: Config, addon: Heroku.AddOn, force = false) { const addonName = addon.name || '' const herokuClient = new APIClient(config) - ux.action.start(`Destroying ${color.addon(addonName)} in the background.\n The app will restart when complete...`) + ux.action.start(`Destroying ${color.addon(addonName)} in the background.\nThe app will restart when complete...`) - const {body: addonDelete} = await herokuClient.delete(`/apps/${addon.app?.id}/addons/${addon.id}`, { + await herokuClient.delete(`/apps/${addon.app?.id}/addons/${addon.id}`, { headers: {'Accept-Expansion': 'plan'}, body: {force}, }).catch(error => { @@ -18,5 +19,4 @@ export default async function (config: any, addon: Heroku.AddOn, force = false) }) ux.action.stop() - return addonDelete } From f908a1999694f0c1e880a64873ba99c2b3651e56 Mon Sep 17 00:00:00 2001 From: Zane Whitfield Date: Wed, 25 Sep 2024 08:11:32 -0700 Subject: [PATCH 06/19] Add confirmation prompt util & update command --- src/commands/ai/models/destroy.ts | 4 +++- src/lib/confirmCommand.ts | 29 +++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 src/lib/confirmCommand.ts diff --git a/src/commands/ai/models/destroy.ts b/src/commands/ai/models/destroy.ts index b063fba..63be467 100644 --- a/src/commands/ai/models/destroy.ts +++ b/src/commands/ai/models/destroy.ts @@ -1,6 +1,7 @@ -import {Args, ux} from '@oclif/core' +import {Args} from '@oclif/core' import {flags} from '@heroku-cli/command' import destroyAddon from '../../../lib/ai/models/destroy_addon' +import confirmCommand from '../../../lib/confirmCommand' import Command from '../../../lib/base' export default class Destroy extends Command { @@ -31,6 +32,7 @@ export default class Destroy extends Command { const aiAddon = this.addon + await confirmCommand(app, confirm) await destroyAddon(this.config, aiAddon, force) } } diff --git a/src/lib/confirmCommand.ts b/src/lib/confirmCommand.ts new file mode 100644 index 0000000..d3b4a2d --- /dev/null +++ b/src/lib/confirmCommand.ts @@ -0,0 +1,29 @@ +import {color} from '@heroku-cli/color' +import {ux} from '@oclif/core' +import heredoc from 'tsheredoc' + +export default async function confirmCommand(app: string, confirm?: string | undefined, message?: string) { + if (confirm) { + if (confirm === app) return + throw new Error(`Confirmation ${color.bold.red(confirm)} did not match ${color.bold.red(app)}. Aborted.`) + } + + if (!message) { + message = heredoc` + Destructive Action. + This command will affect the app ${color.bold.red(app)}. + ` + } + + ux.warn(message) + console.error() + const entered = await ux.prompt( + `To proceed, type ${color.bold.red(app)} or re-run this command with ${color.bold.red('--confirm', app)}`, + {required: true}, + ) + if (entered === app) { + return + } + + throw new Error(`Confirmation did not match ${color.bold.red(app)}. Aborted.`) +} From 7f50160b8d572d4d85c69c81112a75a11c1cb1a1 Mon Sep 17 00:00:00 2001 From: Zane Whitfield Date: Wed, 25 Sep 2024 10:33:43 -0700 Subject: [PATCH 07/19] Update tests Still debugging base.ts test failures --- test/commands/ai/models/destroy.test.ts | 70 +++++++++++++++++++++++++ test/lib/confirmCommand.test.ts | 55 +++++++++++++++++++ 2 files changed, 125 insertions(+) create mode 100644 test/commands/ai/models/destroy.test.ts create mode 100644 test/lib/confirmCommand.test.ts diff --git a/test/commands/ai/models/destroy.test.ts b/test/commands/ai/models/destroy.test.ts new file mode 100644 index 0000000..009ad3a --- /dev/null +++ b/test/commands/ai/models/destroy.test.ts @@ -0,0 +1,70 @@ +import {expect} from 'chai' +import {stderr, stdout} from 'stdout-stderr' +import Cmd from '../../../../src/commands/ai/models/destroy' +import stripAnsi from '../../../helpers/strip-ansi' +import {runCommand} from '../../../run-command' +import {mockAPIErrors, addon1, addon1Attachment1} from '../../../helpers/fixtures' +import {CLIError} from '@oclif/core/lib/errors' +import nock from 'nock' + +describe('ai:models:destroy', function () { + // let herokuAI: nock.Scope + let herokuAPI: nock.Scope + + beforeEach(function () { + // herokuAI = nock('https://inference.heroku.com') + herokuAPI = nock('https://api.heroku.com:443') + }) + + afterEach(function () { + // herokuAI.done() + herokuAPI.done() + nock.cleanAll() + }) + + it('displays confirmation of AI addon destruction', async function () { + // herokuAI + // .get('/available-models') + // .reply(200, availableModels) + const addonAppId = addon1.app?.id + const addonId = addon1.id + const addonName = addon1.name + const appName = addon1.app?.name + + herokuAPI + .post('/actions/addons/resolve', {app: `${appName}`, addon: `${addonName}`}) + .reply(200, [addon1]) + .get(`/addons/${addonId}/addon-attachments`) + .reply(200, [addon1]) + .get(`/apps/${addonAppId}/config-vars`) + .reply(200, { + INFERENCE_KEY: 's3cr3t_k3y', + INFERENCE_MODEL_ID: 'claude-3-5-sonnet', + INFERENCE_URL: 'inference-eu.heroku.com', + }) + .delete(`/apps/${addonAppId}/addons/${addonId}`, {force: false}) + .reply(200, {...addon1, state: 'deprovisioned'}) + + await runCommand(Cmd, [`${addonName}`, '--app', `${appName}`]) + expect(stdout.output).to.contain('test1') + expect(stderr.output).to.eq('test2') + }) + + // it('warns if no models are available', async function () { + // const statusURL = 'https://status.heroku.com/' + // const modelsDevCenterURL = 'https://devcenter.heroku.com/articles/rainbow-unicorn-princess-models' + + // herokuAI + // .get('/available-models') + // .reply(500, mockAPIErrors.modelsListErrorResponse) + + // try { + // await runCommand(Cmd) + // } catch (error) { + // const {message} = error as CLIError + // expect(stripAnsi(message)).to.contains('Failed to retrieve the list of available models.') + // expect(stripAnsi(message)).to.contains(statusURL) + // expect(stripAnsi(message)).to.contains(modelsDevCenterURL) + // } + // }) +}) diff --git a/test/lib/confirmCommand.test.ts b/test/lib/confirmCommand.test.ts new file mode 100644 index 0000000..2d76dca --- /dev/null +++ b/test/lib/confirmCommand.test.ts @@ -0,0 +1,55 @@ +/* eslint-disable mocha/no-setup-in-describe */ +import {ux} from '@oclif/core' +import {expect, test} from '@oclif/test' +import stripAnsi from 'strip-ansi' +import confirmCommand from '../../src/lib/confirmCommand' + +describe('confirmApp', function () { + test + .stdout() + .stderr() + .do(() => confirmCommand('app', 'app')) + .it('should not error or prompt with confirm flag match', ({stderr, stdout}) => { + expect(stderr).to.equal('') + expect(stdout).to.equal('') + }) + + test + .stdout() + .stderr() + .do(() => confirmCommand('app', 'nope')) + .catch((error: Error) => { + expect(stripAnsi(error.message)).to.equal('Confirmation nope did not match app. Aborted.') + }) + .it('should err on confirm flag mismatch') + + test + .stdout() + .stderr() + .stub(ux, 'prompt', () => Promise.resolve('app')) + .do(() => confirmCommand('app')) + .it('should not err on confirm prompt match', ({stderr, stdout}) => { + expect(stderr).to.contain('Warning: Destructive Action') + expect(stdout).to.equal('') + }) + + const customMessage = 'custom message' + + test + .stdout() + .stderr() + .stub(ux, 'prompt', () => Promise.resolve('app')) + .do(() => confirmCommand('app', undefined, customMessage)) + .it('should display custom message', ({stderr, stdout}) => { + expect(stderr).to.contain(customMessage) + expect(stdout).to.equal('') + }) + + test + .stub(ux, 'prompt', () => Promise.resolve('nope')) + .do(() => confirmCommand('app')) + .catch((error: Error) => { + expect(stripAnsi(error.message)).to.equal('Confirmation did not match app. Aborted.') + }) + .it('should err on confirm prompt mismatch') +}) From fe0339e2d1bf373c44815b9e0e909b0720dd0eb3 Mon Sep 17 00:00:00 2001 From: Zane Whitfield Date: Wed, 25 Sep 2024 11:06:11 -0700 Subject: [PATCH 08/19] Update README.md & tests --- README.md | 29 ++++++++++++++++++++++++- test/commands/ai/models/destroy.test.ts | 8 +------ 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 4e29dfd..9292f63 100644 --- a/README.md +++ b/README.md @@ -28,6 +28,7 @@ USAGE * [`heroku ai:docs`](#heroku-aidocs) * [`heroku ai:models`](#heroku-aimodels) * [`heroku ai:models:create MODEL_NAME`](#heroku-aimodelscreate-model_name) +* [`heroku ai:models:destroy MODELRESOURCE`](#heroku-aimodelsdestroy-modelresource) * [`heroku ai:models:list`](#heroku-aimodelslist) ## `heroku ai:docs` @@ -77,7 +78,7 @@ ARGUMENTS MODEL_NAME The name of the model to provision access for FLAGS - -a, --app= (required) The name of the Heroku app to attach the model to + -a, --app= (required) [default: test-cli-plugin-ai2] The name of the Heroku app to attach the model to -r, --remote= git remote of app to use --as= alias name for model resource --confirm= overwrite existing config vars or existing add-on attachments @@ -94,6 +95,32 @@ EXAMPLES _See code: [dist/commands/ai/models/create.ts](https://github.com/heroku/heroku-cli-plugin-integration/blob/v0.0.0/dist/commands/ai/models/create.ts)_ +## `heroku ai:models:destroy MODELRESOURCE` + +destroy an existing AI model resource + +``` +USAGE + $ heroku ai:models:destroy [MODELRESOURCE] -a [-c ] [-f] [-r ] + +ARGUMENTS + MODELRESOURCE The resource ID or alias of the model resource to destroy. + +FLAGS + -a, --app= (required) [default: test-cli-plugin-ai2] app to run command against + -c, --confirm= + -f, --force allow destruction even if connected to other apps + -r, --remote= git remote of app to use + +DESCRIPTION + destroy an existing AI model resource + +EXAMPLES + $ heroku ai:models:destroy claude-3-5-sonnet-acute-43973 +``` + +_See code: [dist/commands/ai/models/destroy.ts](https://github.com/heroku/heroku-cli-plugin-integration/blob/v0.0.0/dist/commands/ai/models/destroy.ts)_ + ## `heroku ai:models:list` list available AI models to provision access to diff --git a/test/commands/ai/models/destroy.test.ts b/test/commands/ai/models/destroy.test.ts index 009ad3a..f4786e4 100644 --- a/test/commands/ai/models/destroy.test.ts +++ b/test/commands/ai/models/destroy.test.ts @@ -3,29 +3,23 @@ import {stderr, stdout} from 'stdout-stderr' import Cmd from '../../../../src/commands/ai/models/destroy' import stripAnsi from '../../../helpers/strip-ansi' import {runCommand} from '../../../run-command' -import {mockAPIErrors, addon1, addon1Attachment1} from '../../../helpers/fixtures' +import {mockAPIErrors, addon1} from '../../../helpers/fixtures' import {CLIError} from '@oclif/core/lib/errors' import nock from 'nock' describe('ai:models:destroy', function () { - // let herokuAI: nock.Scope let herokuAPI: nock.Scope beforeEach(function () { - // herokuAI = nock('https://inference.heroku.com') herokuAPI = nock('https://api.heroku.com:443') }) afterEach(function () { - // herokuAI.done() herokuAPI.done() nock.cleanAll() }) it('displays confirmation of AI addon destruction', async function () { - // herokuAI - // .get('/available-models') - // .reply(200, availableModels) const addonAppId = addon1.app?.id const addonId = addon1.id const addonName = addon1.name From 6611aeb80b4700c58fd99749678ba54328aa37b2 Mon Sep 17 00:00:00 2001 From: Zane Whitfield Date: Wed, 25 Sep 2024 11:12:21 -0700 Subject: [PATCH 09/19] WIP update tests --- test/commands/ai/models/destroy.test.ts | 19 +++++++++---------- test/helpers/fixtures.ts | 6 ++++++ 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/test/commands/ai/models/destroy.test.ts b/test/commands/ai/models/destroy.test.ts index f4786e4..1f68d9d 100644 --- a/test/commands/ai/models/destroy.test.ts +++ b/test/commands/ai/models/destroy.test.ts @@ -3,19 +3,22 @@ import {stderr, stdout} from 'stdout-stderr' import Cmd from '../../../../src/commands/ai/models/destroy' import stripAnsi from '../../../helpers/strip-ansi' import {runCommand} from '../../../run-command' -import {mockAPIErrors, addon1} from '../../../helpers/fixtures' +import {mockConfigVars, addon1} from '../../../helpers/fixtures' import {CLIError} from '@oclif/core/lib/errors' import nock from 'nock' describe('ai:models:destroy', function () { - let herokuAPI: nock.Scope + const {env} = process + let api: nock.Scope beforeEach(function () { - herokuAPI = nock('https://api.heroku.com:443') + process.env = {} + api = nock('https://api.heroku.com:443') }) afterEach(function () { - herokuAPI.done() + process.env = env + api.done() nock.cleanAll() }) @@ -25,17 +28,13 @@ describe('ai:models:destroy', function () { const addonName = addon1.name const appName = addon1.app?.name - herokuAPI + api .post('/actions/addons/resolve', {app: `${appName}`, addon: `${addonName}`}) .reply(200, [addon1]) .get(`/addons/${addonId}/addon-attachments`) .reply(200, [addon1]) .get(`/apps/${addonAppId}/config-vars`) - .reply(200, { - INFERENCE_KEY: 's3cr3t_k3y', - INFERENCE_MODEL_ID: 'claude-3-5-sonnet', - INFERENCE_URL: 'inference-eu.heroku.com', - }) + .reply(200, mockConfigVars) .delete(`/apps/${addonAppId}/addons/${addonId}`, {force: false}) .reply(200, {...addon1, state: 'deprovisioned'}) diff --git a/test/helpers/fixtures.ts b/test/helpers/fixtures.ts index 9a7156c..b4c488b 100644 --- a/test/helpers/fixtures.ts +++ b/test/helpers/fixtures.ts @@ -31,6 +31,12 @@ export const availableModels = [ }, ] +export const mockConfigVars = { + INFERENCE_KEY: 's3cr3t_k3y', + INFERENCE_MODEL_ID: 'claude-3-5-sonnet', + INFERENCE_URL: 'inference-eu.heroku.com', +} + export const mockAPIErrors = { modelsListErrorResponse: { id: 'error', From f6863906d1abe0edbc3062fe7a9248440d53f61d Mon Sep 17 00:00:00 2001 From: Zane Whitfield Date: Wed, 25 Sep 2024 12:14:50 -0700 Subject: [PATCH 10/19] WIP add & update tests --- test/commands/ai/models/destroy.test.ts | 61 ++++++++++++++++--------- test/helpers/fixtures.ts | 4 ++ 2 files changed, 44 insertions(+), 21 deletions(-) diff --git a/test/commands/ai/models/destroy.test.ts b/test/commands/ai/models/destroy.test.ts index 1f68d9d..7677dfc 100644 --- a/test/commands/ai/models/destroy.test.ts +++ b/test/commands/ai/models/destroy.test.ts @@ -3,7 +3,7 @@ import {stderr, stdout} from 'stdout-stderr' import Cmd from '../../../../src/commands/ai/models/destroy' import stripAnsi from '../../../helpers/strip-ansi' import {runCommand} from '../../../run-command' -import {mockConfigVars, addon1} from '../../../helpers/fixtures' +import {mockAPIErrors, mockConfigVars, addon1} from '../../../helpers/fixtures' import {CLIError} from '@oclif/core/lib/errors' import nock from 'nock' @@ -22,6 +22,18 @@ describe('ai:models:destroy', function () { nock.cleanAll() }) + context('no model resource is provided', function () { + it('errors when no model resource is provided', async function () { + try { + await runCommand(Cmd) + } catch (error) { + const {message} = error as CLIError + expect(stripAnsi(message)).contains('Missing 1 required arg:') + expect(stripAnsi(message)).contains('modelResource The resource ID or alias of the model resource to destroy.') + } + }) + }) + it('displays confirmation of AI addon destruction', async function () { const addonAppId = addon1.app?.id const addonId = addon1.id @@ -35,29 +47,36 @@ describe('ai:models:destroy', function () { .reply(200, [addon1]) .get(`/apps/${addonAppId}/config-vars`) .reply(200, mockConfigVars) - .delete(`/apps/${addonAppId}/addons/${addonId}`, {force: false}) - .reply(200, {...addon1, state: 'deprovisioned'}) + // .delete(`/apps/${addonAppId}/addons/${addonId}`, {force: false}) + // .reply(200, {...addon1, state: 'deprovisioned'}) - await runCommand(Cmd, [`${addonName}`, '--app', `${appName}`]) + await runCommand(Cmd, [`${addonName}`, '--app', `${appName}`, '--confirm', `${appName}`]) expect(stdout.output).to.contain('test1') expect(stderr.output).to.eq('test2') }) - // it('warns if no models are available', async function () { - // const statusURL = 'https://status.heroku.com/' - // const modelsDevCenterURL = 'https://devcenter.heroku.com/articles/rainbow-unicorn-princess-models' - - // herokuAI - // .get('/available-models') - // .reply(500, mockAPIErrors.modelsListErrorResponse) - - // try { - // await runCommand(Cmd) - // } catch (error) { - // const {message} = error as CLIError - // expect(stripAnsi(message)).to.contains('Failed to retrieve the list of available models.') - // expect(stripAnsi(message)).to.contains(statusURL) - // expect(stripAnsi(message)).to.contains(modelsDevCenterURL) - // } - // }) + it('displays API error message if destroy request fails', async function () { + const addonAppId = addon1.app?.id + const addonId = addon1.id + const addonName = addon1.name + const appName = addon1.app?.name + + api + .post('/actions/addons/resolve', {app: `${appName}`, addon: `${addonName}`}) + .reply(200, [addon1]) + .get(`/addons/${addonId}/addon-attachments`) + .reply(200, [addon1]) + .get(`/apps/${addonAppId}/config-vars`) + .reply(200, mockConfigVars) + // .delete(`/apps/${addonAppId}/addons/${addonId}`, {force: false}) + // .reply(500, mockAPIErrors.modelsDestroyErrorResponse) + + try { + await runCommand(Cmd, [`${addonName}`, '--app', `${appName}`, '--confirm', `${appName}`]) + } catch (error) { + const {message} = error as CLIError + expect(stripAnsi(message)).to.contains('The add-on was unable to be destroyed:') + expect(stripAnsi(message)).to.contains(mockAPIErrors.modelsDestroyErrorResponse.message) + } + }) }) diff --git a/test/helpers/fixtures.ts b/test/helpers/fixtures.ts index b4c488b..0ff4489 100644 --- a/test/helpers/fixtures.ts +++ b/test/helpers/fixtures.ts @@ -42,6 +42,10 @@ export const mockAPIErrors = { id: 'error', message: 'Failed to retrieve the list of available models. Check the Heroku Status page https://status.heroku.com/ for system outages. After all incidents have resolved, try again. You can also see a list of models at https://devcenter.heroku.com/articles/rainbow-unicorn-princess-models.', }, + modelsDestroyErrorResponse: { + id: 'error', + message: 'Example API Error', + }, } export const addon1: Heroku.AddOn = { From 71530afddc8faa99d94f7c56062bd896562e6def Mon Sep 17 00:00:00 2001 From: Zane Whitfield Date: Wed, 25 Sep 2024 12:19:33 -0700 Subject: [PATCH 11/19] Update README.md --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 9292f63..fcef6ef 100644 --- a/README.md +++ b/README.md @@ -78,7 +78,7 @@ ARGUMENTS MODEL_NAME The name of the model to provision access for FLAGS - -a, --app= (required) [default: test-cli-plugin-ai2] The name of the Heroku app to attach the model to + -a, --app= (required) The name of the Heroku app to attach the model to -r, --remote= git remote of app to use --as= alias name for model resource --confirm= overwrite existing config vars or existing add-on attachments @@ -107,7 +107,7 @@ ARGUMENTS MODELRESOURCE The resource ID or alias of the model resource to destroy. FLAGS - -a, --app= (required) [default: test-cli-plugin-ai2] app to run command against + -a, --app= (required) app to run command against -c, --confirm= -f, --force allow destruction even if connected to other apps -r, --remote= git remote of app to use From f95a05ed260f93800c29c7fab46dfd2fd8644599 Mon Sep 17 00:00:00 2001 From: Zane Whitfield Date: Wed, 25 Sep 2024 14:36:51 -0700 Subject: [PATCH 12/19] Update mockConfigVars --- test/helpers/fixtures.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/test/helpers/fixtures.ts b/test/helpers/fixtures.ts index 0ff4489..05d0be2 100644 --- a/test/helpers/fixtures.ts +++ b/test/helpers/fixtures.ts @@ -31,10 +31,18 @@ export const availableModels = [ }, ] -export const mockConfigVars = { - INFERENCE_KEY: 's3cr3t_k3y', - INFERENCE_MODEL_ID: 'claude-3-5-sonnet', - INFERENCE_URL: 'inference-eu.heroku.com', +export const configureMockConfigVars = (addonAttachment: Heroku.AddOnAttachment) => { + const updatedMockConfigVars: any = {} + + const newInferenceKey = `${addonAttachment.addon?.name.toUpperCase()}_KEY` + const newInferenceURL = `${addonAttachment.addon?.name.toUpperCase()}_URL` + const newInferenceMODELID = `${addonAttachment.addon?.name.toUpperCase()}_MODEL_ID` + + updatedMockConfigVars[newInferenceKey] = 's3cr3t_k3y' + updatedMockConfigVars[newInferenceMODELID] = 'claude-3-5-sonnet' + updatedMockConfigVars[newInferenceURL] = 'inference-eu.heroku.com' + + return updatedMockConfigVars } export const mockAPIErrors = { From 49a4e751f22b7c3ad9e9a02c3348ab1b3d8da379 Mon Sep 17 00:00:00 2001 From: Zane Whitfield Date: Wed, 25 Sep 2024 14:37:27 -0700 Subject: [PATCH 13/19] Debug tests --- test/commands/ai/models/destroy.test.ts | 51 +++++++++++++------------ 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/test/commands/ai/models/destroy.test.ts b/test/commands/ai/models/destroy.test.ts index 7677dfc..820dcf9 100644 --- a/test/commands/ai/models/destroy.test.ts +++ b/test/commands/ai/models/destroy.test.ts @@ -3,13 +3,14 @@ import {stderr, stdout} from 'stdout-stderr' import Cmd from '../../../../src/commands/ai/models/destroy' import stripAnsi from '../../../helpers/strip-ansi' import {runCommand} from '../../../run-command' -import {mockAPIErrors, mockConfigVars, addon1} from '../../../helpers/fixtures' +import {configureMockConfigVars, mockAPIErrors, addon1, addon1Attachment1} from '../../../helpers/fixtures' import {CLIError} from '@oclif/core/lib/errors' import nock from 'nock' describe('ai:models:destroy', function () { const {env} = process let api: nock.Scope + const configuredMockConfigVars = configureMockConfigVars(addon1Attachment1) beforeEach(function () { process.env = {} @@ -46,37 +47,37 @@ describe('ai:models:destroy', function () { .get(`/addons/${addonId}/addon-attachments`) .reply(200, [addon1]) .get(`/apps/${addonAppId}/config-vars`) - .reply(200, mockConfigVars) + .reply(200, configuredMockConfigVars) // .delete(`/apps/${addonAppId}/addons/${addonId}`, {force: false}) // .reply(200, {...addon1, state: 'deprovisioned'}) - await runCommand(Cmd, [`${addonName}`, '--app', `${appName}`, '--confirm', `${appName}`]) + await runCommand(Cmd, [`${addonName}`, '--app', `${appName}`, '--confirm', `${appName}`], true) expect(stdout.output).to.contain('test1') expect(stderr.output).to.eq('test2') }) - it('displays API error message if destroy request fails', async function () { - const addonAppId = addon1.app?.id - const addonId = addon1.id - const addonName = addon1.name - const appName = addon1.app?.name + // it('displays API error message if destroy request fails', async function () { + // const addonAppId = addon1.app?.id + // const addonId = addon1.id + // const addonName = addon1.name + // const appName = addon1.app?.name - api - .post('/actions/addons/resolve', {app: `${appName}`, addon: `${addonName}`}) - .reply(200, [addon1]) - .get(`/addons/${addonId}/addon-attachments`) - .reply(200, [addon1]) - .get(`/apps/${addonAppId}/config-vars`) - .reply(200, mockConfigVars) - // .delete(`/apps/${addonAppId}/addons/${addonId}`, {force: false}) - // .reply(500, mockAPIErrors.modelsDestroyErrorResponse) + // api + // .post('/actions/addons/resolve', {app: `${appName}`, addon: `${addonName}`}) + // .reply(200, [addon1]) + // .get(`/addons/${addonId}/addon-attachments`) + // .reply(200, [addon1]) + // .get(`/apps/${addonAppId}/config-vars`) + // .reply(200, configuredMockConfigVars) + // // .delete(`/apps/${addonAppId}/addons/${addonId}`, {force: false}) + // // .reply(500, mockAPIErrors.modelsDestroyErrorResponse) - try { - await runCommand(Cmd, [`${addonName}`, '--app', `${appName}`, '--confirm', `${appName}`]) - } catch (error) { - const {message} = error as CLIError - expect(stripAnsi(message)).to.contains('The add-on was unable to be destroyed:') - expect(stripAnsi(message)).to.contains(mockAPIErrors.modelsDestroyErrorResponse.message) - } - }) + // try { + // await runCommand(Cmd, [`${addonName}`, '--app', `${appName}`, '--confirm', `${appName}`]) + // } catch (error) { + // const {message} = error as CLIError + // expect(stripAnsi(message)).to.contains('The add-on was unable to be destroyed:') + // expect(stripAnsi(message)).to.contains(mockAPIErrors.modelsDestroyErrorResponse.message) + // } + // }) }) From f6dba5a8ca6b67486ad04b2ab1c822baa6413efa Mon Sep 17 00:00:00 2001 From: Zane Whitfield Date: Wed, 25 Sep 2024 14:47:20 -0700 Subject: [PATCH 14/19] Fix first test failure --- test/commands/ai/models/destroy.test.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test/commands/ai/models/destroy.test.ts b/test/commands/ai/models/destroy.test.ts index 820dcf9..ec4a7ae 100644 --- a/test/commands/ai/models/destroy.test.ts +++ b/test/commands/ai/models/destroy.test.ts @@ -48,12 +48,13 @@ describe('ai:models:destroy', function () { .reply(200, [addon1]) .get(`/apps/${addonAppId}/config-vars`) .reply(200, configuredMockConfigVars) - // .delete(`/apps/${addonAppId}/addons/${addonId}`, {force: false}) - // .reply(200, {...addon1, state: 'deprovisioned'}) + .delete(`/apps/${addonAppId}/addons/${addonId}`, {force: false}) + .reply(200, {...addon1, state: 'deprovisioned'}) - await runCommand(Cmd, [`${addonName}`, '--app', `${appName}`, '--confirm', `${appName}`], true) - expect(stdout.output).to.contain('test1') - expect(stderr.output).to.eq('test2') + await runCommand(Cmd, [`${addonName}`, '--app', `${appName}`, '--confirm', `${appName}`]) + expect(stderr.output).contains(`Destroying ${addonName} in the background.`) + expect(stderr.output).contains('The app will restart when complete...') + expect(stdout.output).to.eq('') }) // it('displays API error message if destroy request fails', async function () { From 810240023b47b68838567b96be79553880126bb8 Mon Sep 17 00:00:00 2001 From: Zane Whitfield Date: Thu, 26 Sep 2024 11:05:05 -0700 Subject: [PATCH 15/19] Update tests & remove remote Remote can affect tests --- test/commands/ai/models/destroy.test.ts | 44 ++++++++++++------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/test/commands/ai/models/destroy.test.ts b/test/commands/ai/models/destroy.test.ts index ec4a7ae..e5c9860 100644 --- a/test/commands/ai/models/destroy.test.ts +++ b/test/commands/ai/models/destroy.test.ts @@ -57,28 +57,28 @@ describe('ai:models:destroy', function () { expect(stdout.output).to.eq('') }) - // it('displays API error message if destroy request fails', async function () { - // const addonAppId = addon1.app?.id - // const addonId = addon1.id - // const addonName = addon1.name - // const appName = addon1.app?.name + it('displays API error message if destroy request fails', async function () { + const addonAppId = addon1.app?.id + const addonId = addon1.id + const addonName = addon1.name + const appName = addon1.app?.name - // api - // .post('/actions/addons/resolve', {app: `${appName}`, addon: `${addonName}`}) - // .reply(200, [addon1]) - // .get(`/addons/${addonId}/addon-attachments`) - // .reply(200, [addon1]) - // .get(`/apps/${addonAppId}/config-vars`) - // .reply(200, configuredMockConfigVars) - // // .delete(`/apps/${addonAppId}/addons/${addonId}`, {force: false}) - // // .reply(500, mockAPIErrors.modelsDestroyErrorResponse) + api + .post('/actions/addons/resolve', {app: `${appName}`, addon: `${addonName}`}) + .reply(200, [addon1]) + .get(`/addons/${addonId}/addon-attachments`) + .reply(200, [addon1]) + .get(`/apps/${addonAppId}/config-vars`) + .reply(200, configuredMockConfigVars) + .delete(`/apps/${addonAppId}/addons/${addonId}`, {force: false}) + .reply(500, mockAPIErrors.modelsDestroyErrorResponse) - // try { - // await runCommand(Cmd, [`${addonName}`, '--app', `${appName}`, '--confirm', `${appName}`]) - // } catch (error) { - // const {message} = error as CLIError - // expect(stripAnsi(message)).to.contains('The add-on was unable to be destroyed:') - // expect(stripAnsi(message)).to.contains(mockAPIErrors.modelsDestroyErrorResponse.message) - // } - // }) + try { + await runCommand(Cmd, [`${addonName}`, '--app', `${appName}`, '--confirm', `${appName}`]) + } catch (error) { + const {message} = error as CLIError + expect(stripAnsi(message)).to.contains('The add-on was unable to be destroyed:') + expect(stripAnsi(message)).to.contains(mockAPIErrors.modelsDestroyErrorResponse.message) + } + }) }) From 786409d3a7480044dfe4972bed055ce7c769a203 Mon Sep 17 00:00:00 2001 From: Zane Whitfield Date: Thu, 26 Sep 2024 11:20:03 -0700 Subject: [PATCH 16/19] Add ux.action.stop() in try-catch Without ux.action.stop() before error incatch, the test will swallow the output --- src/lib/ai/models/destroy_addon.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib/ai/models/destroy_addon.ts b/src/lib/ai/models/destroy_addon.ts index 39d5465..8ed468a 100644 --- a/src/lib/ai/models/destroy_addon.ts +++ b/src/lib/ai/models/destroy_addon.ts @@ -14,6 +14,7 @@ export default async function (config: Config, addon: Heroku.AddOn, force = fals headers: {'Accept-Expansion': 'plan'}, body: {force}, }).catch(error => { + ux.action.stop('') const error_ = error.body && error.body.message ? new Error(`The add-on was unable to be destroyed: ${error.body.message}.`) : new Error(`The add-on was unable to be destroyed: ${error}.`) throw error_ }) From 7166f797bf75719f2dcf1c04591916cca298b2da Mon Sep 17 00:00:00 2001 From: Zane Whitfield Date: Thu, 26 Sep 2024 11:36:02 -0700 Subject: [PATCH 17/19] Update tests --- test/commands/ai/models/destroy.test.ts | 11 +++++------ test/helpers/fixtures.ts | 16 ++++------------ 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/test/commands/ai/models/destroy.test.ts b/test/commands/ai/models/destroy.test.ts index e5c9860..a8edf63 100644 --- a/test/commands/ai/models/destroy.test.ts +++ b/test/commands/ai/models/destroy.test.ts @@ -3,14 +3,13 @@ import {stderr, stdout} from 'stdout-stderr' import Cmd from '../../../../src/commands/ai/models/destroy' import stripAnsi from '../../../helpers/strip-ansi' import {runCommand} from '../../../run-command' -import {configureMockConfigVars, mockAPIErrors, addon1, addon1Attachment1} from '../../../helpers/fixtures' +import {mockConfigVars, mockAPIErrors, addon1, addon1Attachment1} from '../../../helpers/fixtures' import {CLIError} from '@oclif/core/lib/errors' import nock from 'nock' describe('ai:models:destroy', function () { const {env} = process let api: nock.Scope - const configuredMockConfigVars = configureMockConfigVars(addon1Attachment1) beforeEach(function () { process.env = {} @@ -45,9 +44,9 @@ describe('ai:models:destroy', function () { .post('/actions/addons/resolve', {app: `${appName}`, addon: `${addonName}`}) .reply(200, [addon1]) .get(`/addons/${addonId}/addon-attachments`) - .reply(200, [addon1]) + .reply(200, [addon1Attachment1]) .get(`/apps/${addonAppId}/config-vars`) - .reply(200, configuredMockConfigVars) + .reply(200, mockConfigVars) .delete(`/apps/${addonAppId}/addons/${addonId}`, {force: false}) .reply(200, {...addon1, state: 'deprovisioned'}) @@ -67,9 +66,9 @@ describe('ai:models:destroy', function () { .post('/actions/addons/resolve', {app: `${appName}`, addon: `${addonName}`}) .reply(200, [addon1]) .get(`/addons/${addonId}/addon-attachments`) - .reply(200, [addon1]) + .reply(200, [addon1Attachment1]) .get(`/apps/${addonAppId}/config-vars`) - .reply(200, configuredMockConfigVars) + .reply(200, mockConfigVars) .delete(`/apps/${addonAppId}/addons/${addonId}`, {force: false}) .reply(500, mockAPIErrors.modelsDestroyErrorResponse) diff --git a/test/helpers/fixtures.ts b/test/helpers/fixtures.ts index 05d0be2..3d5a178 100644 --- a/test/helpers/fixtures.ts +++ b/test/helpers/fixtures.ts @@ -31,18 +31,10 @@ export const availableModels = [ }, ] -export const configureMockConfigVars = (addonAttachment: Heroku.AddOnAttachment) => { - const updatedMockConfigVars: any = {} - - const newInferenceKey = `${addonAttachment.addon?.name.toUpperCase()}_KEY` - const newInferenceURL = `${addonAttachment.addon?.name.toUpperCase()}_URL` - const newInferenceMODELID = `${addonAttachment.addon?.name.toUpperCase()}_MODEL_ID` - - updatedMockConfigVars[newInferenceKey] = 's3cr3t_k3y' - updatedMockConfigVars[newInferenceMODELID] = 'claude-3-5-sonnet' - updatedMockConfigVars[newInferenceURL] = 'inference-eu.heroku.com' - - return updatedMockConfigVars +export const mockConfigVars = { + INFERENCE_KEY: 's3cr3t_k3y', + INFERENCE_MODEL_ID: 'claude-3-opus', + INFERENCE_URL: 'inference-eu.heroku.com', } export const mockAPIErrors = { From 9a5a60beea5867c252a15163c53d46b86cf5a95c Mon Sep 17 00:00:00 2001 From: Zane Whitfield Date: Thu, 26 Sep 2024 15:42:09 -0700 Subject: [PATCH 18/19] Add error to base.ts for PR feedback --- src/lib/base.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/lib/base.ts b/src/lib/base.ts index f5a835c..74261f2 100644 --- a/src/lib/base.ts +++ b/src/lib/base.ts @@ -19,6 +19,18 @@ export class NotFound extends Error { public readonly id = 'not_found' } +export class AppNotFound extends Error { + constructor(appIdentifier?: string) { + const message = heredoc` + We can’t find the ${color.app(appIdentifier)} app. Check your spelling. + ` + super(message) + } + + public readonly statusCode = 404 + public readonly id = 'not_found' +} + export class AmbiguousError extends Error { constructor(public readonly matches: string[], addonIdentifier: string, appIdentifier?: string) { const message = heredoc` @@ -211,11 +223,17 @@ export default abstract class extends Command { const attachmentNotFound = attachmentResolverError instanceof HerokuAPIError && attachmentResolverError.http.statusCode === 404 && attachmentResolverError.body.resource === 'add_on attachment' + const appNotFound = attachmentResolverError instanceof HerokuAPIError && + attachmentResolverError.http.statusCode === 404 && + attachmentResolverError.body.resource === 'app' let error = addonResolverError if (addonNotFound) error = attachmentNotFound ? new NotFound(addonIdentifier, appIdentifier) : attachmentResolverError + if (appNotFound) + error = new AppNotFound(appIdentifier) + throw error } From 38c15b742d57c861416585acc29d3e9cb007e26e Mon Sep 17 00:00:00 2001 From: Zane Whitfield Date: Thu, 26 Sep 2024 16:53:53 -0700 Subject: [PATCH 19/19] Add test for new error --- test/lib/base.test.ts | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/lib/base.test.ts b/test/lib/base.test.ts index aa401a4..7a2b0b8 100644 --- a/test/lib/base.test.ts +++ b/test/lib/base.test.ts @@ -188,6 +188,32 @@ describe('attempt a request using the Heroku AI client', function () { }) }) + context('when using an existent model resource name with non-existent app', function () { + beforeEach(async function () { + api + .post('/actions/addons/resolve', {addon: addon1.name, app: 'app2'}) + .reply(404, [addon1]) + .post('/actions/addon-attachments/resolve', {addon_attachment: addon1.name, app: 'app2'}) + .reply(404, {id: 'not_found', message: 'Couldn\'t find that app.', resource: 'app'}) + }) + + it('returns a custom not found error message', async function () { + try { + await runCommand(CommandConfiguredWithResourceName, [ + addon1.name as string, + '--app=app2', + ]) + } catch (error) { + const {message} = error as Error + expect(stripAnsi(message)).to.equal(heredoc` + We can’t find the app2 app. Check your spelling. + `) + } + + expect(stdout.output).to.equal('') + }) + }) + context('when using the add-on service slug and no app, matching multiple model resources', function () { beforeEach(async function () { api