From d51f6adcf6aefd30a6bb4f16b09e64ae6e36f2b3 Mon Sep 17 00:00:00 2001 From: axel7083 <42176370+axel7083@users.noreply.github.com> Date: Mon, 22 Apr 2024 11:48:30 +0200 Subject: [PATCH] fix: tests Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> --- .../src/managers/applicationManager.spec.ts | 7 +- .../src/managers/applicationManager.ts | 4 +- .../backend/src/managers/catalogManager.ts | 7 +- packages/backend/src/managers/gitManager.ts | 3 + .../inference/inferenceManager.spec.ts | 6 +- .../backend/src/managers/modelsManager.ts | 66 ++++++++++--------- packages/backend/src/studio-api-impl.ts | 4 +- packages/backend/src/utils/podman.spec.ts | 3 +- packages/backend/src/utils/podman.ts | 15 ++++- packages/shared/src/StudioAPI.ts | 2 +- 10 files changed, 69 insertions(+), 48 deletions(-) diff --git a/packages/backend/src/managers/applicationManager.spec.ts b/packages/backend/src/managers/applicationManager.spec.ts index 4b60d5d25..8c1aa5502 100644 --- a/packages/backend/src/managers/applicationManager.spec.ts +++ b/packages/backend/src/managers/applicationManager.spec.ts @@ -278,7 +278,6 @@ describe('pullApplication', () => { ref: '000000', readme: '', repository: 'repo', - config: 'ai-lab.yaml', }; const model: ModelInfo = { id: 'model1', @@ -287,7 +286,7 @@ describe('pullApplication', () => { license: '', name: 'Model 1', registry: '', - url: '', + url: 'dummy-url', memory: 1000, }; mocks.inspectContainerMock.mockResolvedValue({ @@ -343,7 +342,7 @@ describe('pullApplication', () => { description: '', readme: '', repository: 'repo', - config: 'ai-lab.yaml', + basedir: 'ai-lab.yaml', }; const model: ModelInfo = { id: 'model1', @@ -372,7 +371,7 @@ describe('pullApplication', () => { ref: '000000', readme: '', repository: 'repo', - config: 'ai-lab.yaml', + basedir: 'ai-lab.yaml', }; const model: ModelInfo = { id: 'model1', diff --git a/packages/backend/src/managers/applicationManager.ts b/packages/backend/src/managers/applicationManager.ts index eae3f3a39..17d56bbe5 100644 --- a/packages/backend/src/managers/applicationManager.ts +++ b/packages/backend/src/managers/applicationManager.ts @@ -501,7 +501,7 @@ export class ApplicationManager extends Publisher implements } getConfigAndFilterContainers( - recipeBaseDir: string, + recipeBaseDir: string | undefined, localFolder: string, labels?: { [key: string]: string }, ): AIContainers { @@ -543,7 +543,7 @@ export class ApplicationManager extends Publisher implements ); } - getConfiguration(recipeBaseDir: string, localFolder: string): AIConfigFile { + getConfiguration(recipeBaseDir: string | undefined, localFolder: string): AIConfigFile { let configFile: string; if (recipeBaseDir !== undefined) { configFile = path.join(localFolder, recipeBaseDir, CONFIG_FILENAME); diff --git a/packages/backend/src/managers/catalogManager.ts b/packages/backend/src/managers/catalogManager.ts index 10daf9ec8..2ac211b74 100644 --- a/packages/backend/src/managers/catalogManager.ts +++ b/packages/backend/src/managers/catalogManager.ts @@ -70,7 +70,7 @@ export class CatalogManager extends Publisher implements Dis } private loadDefaultCatalog(): void { - this.catalog = defaultCatalog; + this.catalog = defaultCatalog as ApplicationCatalog; this.notify(); } @@ -82,7 +82,10 @@ export class CatalogManager extends Publisher implements Dis const sanitize = this.sanitize(content); this.catalog = { - models: [...defaultCatalog.models.filter(a => !sanitize.models.some(b => a.id === b.id)), ...sanitize.models], + models: [ + ...defaultCatalog.models.filter(a => !sanitize.models.some(b => a.id === b.id)), + ...sanitize.models, + ] as ModelInfo[], recipes: [...defaultCatalog.recipes.filter(a => !sanitize.recipes.some(b => a.id === b.id)), ...sanitize.recipes], categories: [ ...defaultCatalog.categories.filter(a => !sanitize.categories.some(b => a.id === b.id)), diff --git a/packages/backend/src/managers/gitManager.ts b/packages/backend/src/managers/gitManager.ts index 9395dd2e5..271cc1b2b 100644 --- a/packages/backend/src/managers/gitManager.ts +++ b/packages/backend/src/managers/gitManager.ts @@ -275,6 +275,9 @@ export class GitManager { while (remoteCommits.length && localCommits.length) { const remote = remoteCommits.pop(); const local = localCommits.pop(); + if (!remote || !local) { + break; + } if (remote === local) { continue; } diff --git a/packages/backend/src/managers/inference/inferenceManager.spec.ts b/packages/backend/src/managers/inference/inferenceManager.spec.ts index e8c31839a..9ede5bb3f 100644 --- a/packages/backend/src/managers/inference/inferenceManager.spec.ts +++ b/packages/backend/src/managers/inference/inferenceManager.spec.ts @@ -565,7 +565,7 @@ describe('containerRegistry events', () => { const listener = await deferred; const server = inferenceManager.get('dummyId'); - expect(server.status).toBe('running'); + expect(server?.status).toBe('running'); expect(containerEngine.inspectContainer).toHaveBeenCalledOnce(); vi.mocked(containerEngine.inspectContainer).mockResolvedValue({ @@ -578,7 +578,7 @@ describe('containerRegistry events', () => { listener('die'); await vi.waitFor(() => { - expect(inferenceManager.get('dummyId').status).toBe('stopped'); + expect(inferenceManager.get('dummyId')?.status).toBe('stopped'); expect(containerEngine.inspectContainer).toHaveBeenCalledTimes(2); }); @@ -611,7 +611,7 @@ describe('containerRegistry events', () => { const listener = await deferred; const server = inferenceManager.get('dummyId'); - expect(server.status).toBe('running'); + expect(server?.status).toBe('running'); listener('remove'); diff --git a/packages/backend/src/managers/modelsManager.ts b/packages/backend/src/managers/modelsManager.ts index 84fa1ac01..dc1bbb946 100644 --- a/packages/backend/src/managers/modelsManager.ts +++ b/packages/backend/src/managers/modelsManager.ts @@ -171,38 +171,40 @@ export class ModelsManager implements Disposable { async deleteModel(modelId: string): Promise { const model = this.#models.get(modelId); - if (model) { - model.state = 'deleting'; - await this.sendModelsInfo(); - try { - await this.deleteRemoteModel(model); - let modelPath; - // if model does not have any url, it has been imported locally by the user - if (!model.url) { - modelPath = path.join(model.file.path, model.file.file); - // remove it from the catalog as it cannot be downloaded anymore - await this.catalogManager.removeUserModel(modelId); - } else { - modelPath = this.getLocalModelFolder(modelId); - } - await fs.promises.rm(modelPath, { recursive: true, force: true, maxRetries: 3 }); - - this.telemetry.logUsage('model.delete', { 'model.id': modelId }); - model.file = model.state = undefined; - } catch (err: unknown) { - this.telemetry.logError('model.delete', { - 'model.id': modelId, - message: 'error deleting model from disk', - error: err, - }); - await podmanDesktopApi.window.showErrorMessage(`Error deleting model ${modelId}. ${String(err)}`); + if (!model?.file) { + throw new Error('model cannot be found.'); + } - // Let's reload the models manually to avoid any issue - model.state = undefined; - this.getLocalModelsFromDisk(); - } finally { - await this.sendModelsInfo(); + model.state = 'deleting'; + await this.sendModelsInfo(); + try { + await this.deleteRemoteModel(model); + let modelPath; + // if model does not have any url, it has been imported locally by the user + if (!model.url) { + modelPath = path.join(model.file.path, model.file.file); + // remove it from the catalog as it cannot be downloaded anymore + await this.catalogManager.removeUserModel(modelId); + } else { + modelPath = this.getLocalModelFolder(modelId); } + await fs.promises.rm(modelPath, { recursive: true, force: true, maxRetries: 3 }); + + this.telemetry.logUsage('model.delete', { 'model.id': modelId }); + model.file = model.state = undefined; + } catch (err: unknown) { + this.telemetry.logError('model.delete', { + 'model.id': modelId, + message: 'error deleting model from disk', + error: err, + }); + await podmanDesktopApi.window.showErrorMessage(`Error deleting model ${modelId}. ${String(err)}`); + + // Let's reload the models manually to avoid any issue + model.state = undefined; + this.getLocalModelsFromDisk(); + } finally { + await this.sendModelsInfo(); } } @@ -326,6 +328,10 @@ export class ModelsManager implements Disposable { } private createDownloader(model: ModelInfo, abortSignal: AbortSignal): Downloader { + if (!model.url) { + throw new Error(`model ${model.id} does not have url defined.`); + } + // Ensure path to model directory exist const destDir = path.join(this.appUserDirectory, 'models', model.id); if (!fs.existsSync(destDir)) { diff --git a/packages/backend/src/studio-api-impl.ts b/packages/backend/src/studio-api-impl.ts index 58e71eee4..46cf82bad 100644 --- a/packages/backend/src/studio-api-impl.ts +++ b/packages/backend/src/studio-api-impl.ts @@ -178,7 +178,7 @@ export class StudioApiImpl implements StudioAPI { } } - async openDialog(options?: podmanDesktopApi.OpenDialogOptions): Promise { + async openDialog(options?: podmanDesktopApi.OpenDialogOptions): Promise { return await podmanDesktopApi.window.showOpenDialog(options); } @@ -464,7 +464,7 @@ export class StudioApiImpl implements StudioAPI { } async checkInvalidModels(models: string[]): Promise { - const invalidPaths = []; + const invalidPaths: string[] = []; const catalogModels = await this.getModelsInfo(); for (const model of models) { const modelPath = path.resolve(model); diff --git a/packages/backend/src/utils/podman.spec.ts b/packages/backend/src/utils/podman.spec.ts index 8e2d446bd..a47bc4419 100644 --- a/packages/backend/src/utils/podman.spec.ts +++ b/packages/backend/src/utils/podman.spec.ts @@ -19,6 +19,7 @@ import { beforeEach, expect, test, describe, vi } from 'vitest'; import * as podmanDesktopApi from '@podman-desktop/api'; import * as utils from '../utils/podman'; +import type { ContainerEngineInfo } from '@podman-desktop/api'; const mocks = vi.hoisted(() => { return { @@ -354,7 +355,7 @@ describe('checkContainerConnectionStatusAndResources', () => { providerId: 'podman', }, ]); - vi.mocked(podmanDesktopApi.containerEngine.info).mockResolvedValue(undefined); + vi.mocked(podmanDesktopApi.containerEngine.info).mockResolvedValue(undefined as unknown as ContainerEngineInfo); const result = await utils.checkContainerConnectionStatusAndResources({ memoryNeeded: 10, context: 'inference', diff --git a/packages/backend/src/utils/podman.ts b/packages/backend/src/utils/podman.ts index fe0dc4361..0ae61d46d 100644 --- a/packages/backend/src/utils/podman.ts +++ b/packages/backend/src/utils/podman.ts @@ -138,7 +138,7 @@ export async function isQEMUMachine(): Promise { export async function checkContainerConnectionStatusAndResources( modelInfo: ModelCheckerInfo, ): Promise { - let connection: ProviderContainerConnection; + let connection: ProviderContainerConnection | undefined = undefined; try { connection = getFirstRunningPodmanConnection(); } catch (e) { @@ -166,13 +166,22 @@ export async function checkContainerConnectionStatusAndResources( const hasCpus = engineInfo.cpus !== undefined && engineInfo.cpus >= MIN_CPUS_VALUE; const multiplier = modelInfo.context === 'recipe' ? 1.25 : 1.1; const memoryExpected = modelInfo.memoryNeeded * multiplier; - const hasMemory = engineInfo.memory - engineInfo.memoryUsed >= memoryExpected; + + let hasMemory: boolean = true; + if (engineInfo.memory !== undefined && engineInfo.memoryUsed !== undefined) { + hasMemory = engineInfo.memory - engineInfo.memoryUsed >= memoryExpected; + } + + let memoryIdle: number = 0; + if (engineInfo.memory !== undefined && engineInfo.memoryUsed !== undefined) { + memoryIdle = engineInfo.memory - engineInfo.memoryUsed; + } if (!hasCpus || !hasMemory) { return { name: connection.connection.name, cpus: engineInfo.cpus ?? 0, - memoryIdle: engineInfo.memory - engineInfo.memoryUsed, + memoryIdle: memoryIdle, cpusExpected: MIN_CPUS_VALUE, memoryExpected: memoryExpected, status: 'low-resources', diff --git a/packages/shared/src/StudioAPI.ts b/packages/shared/src/StudioAPI.ts index a1e0056c4..a3248818f 100644 --- a/packages/shared/src/StudioAPI.ts +++ b/packages/shared/src/StudioAPI.ts @@ -37,7 +37,7 @@ export abstract class StudioAPI { abstract pullApplication(recipeId: string, modelId: string): Promise; abstract openURL(url: string): Promise; abstract openFile(file: string, recipeId?: string): Promise; - abstract openDialog(options?: OpenDialogOptions): Promise; + abstract openDialog(options?: OpenDialogOptions): Promise; /** * Get the information of models saved locally into the user's directory