Skip to content

feat: add upgrade model support in fileManager remove operation #138

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rsonghuster merged 1 commit into from
Jan 24, 2026
Merged

feat: add upgrade model support in fileManager remove operation #138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
191 changes: 92 additions & 99 deletions __tests__/ut/commands/artModelService_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { ArtModelService } from '../../../src/subCommands/model/fileManager';
import { IInputs } from '../../../src/interface';
import DevClient from '@alicloud/devs20230714';
import { sleep } from '../../../src/utils';
import { initClient, checkModelStatus } from '../../../src/subCommands/model/utils';
import { initClient } from '../../../src/subCommands/model/utils';

// Mock dependencies
jest.mock('../../../src/logger', () => {
Expand Down Expand Up @@ -33,11 +33,13 @@ jest.mock('../../../src/utils');
jest.mock('../../../src/subCommands/model/utils', () => {
const originalModule = jest.requireActual('../../../src/subCommands/model/utils');
const mockRetryFileManagerRsyncAndCheckStatus = jest.fn();
const mockRetryFileManagerRm = jest.fn();
return {
__esModule: true,
...originalModule,
retryWithFileManager: jest.fn((command, fn) => fn()),
retryFileManagerRsyncAndCheckStatus: mockRetryFileManagerRsyncAndCheckStatus,
retryFileManagerRm: mockRetryFileManagerRm,
initClient: jest.fn(),
checkModelStatus: jest.fn(),
extractOssMountDir: jest.fn(),
Expand Down Expand Up @@ -81,6 +83,13 @@ describe('ArtModelService', () => {
};

artModelService = new ArtModelService(mockInputs);
// 添加 createResource 属性以防止错误
Object.defineProperty(artModelService, 'createResource', {
value: { history: [], oss: {}, nas: {}, vpc: {}, sls: {} },
writable: true,
enumerable: true,
configurable: true,
});
});

afterEach(() => {
Expand Down Expand Up @@ -203,8 +212,8 @@ describe('ArtModelService', () => {
},
],
},
storage: 'nas',
nasMountPoints: [{ mountDir: '/mnt/test' }],
ossMountPoints: [{ mountDir: '/mnt/oss' }],
role: 'acs:ram::123456789:role/aliyundevsdefaultrole',
region: 'cn-hangzhou',
vpcConfig: {},
Expand All @@ -218,21 +227,19 @@ describe('ArtModelService', () => {
},
} as any);

mockDevClient.fileManagerRsync.mockResolvedValue({
body: {
success: true,
data: {
taskID: 'task-123',
},
requestId: 'req-123',
},
} as any);

// ArtModelService内部会调用checkModelStatus，所以我们需要模拟它
(checkModelStatus as jest.Mock).mockResolvedValue(undefined);
// 现在使用重试函数，我们需要模拟重试函数成功执行
(
require('../../../src/subCommands/model/utils')
.retryFileManagerRsyncAndCheckStatus as jest.Mock
).mockResolvedValue(undefined);

// 成功下载应该正常完成而不抛出异常
await expect(artModelService.downloadModel(name, params)).resolves.toBeUndefined();

// 验证重试函数被调用
expect(
require('../../../src/subCommands/model/utils').retryFileManagerRsyncAndCheckStatus,
).toHaveBeenCalled();
});

it('should handle download error from fileManagerRsync', async () => {
Expand All @@ -253,8 +260,8 @@ describe('ArtModelService', () => {
},
],
},
storage: 'nas',
nasMountPoints: [{ mountDir: '/mnt/test' }],
ossMountPoints: [{ mountDir: '/mnt/oss' }],
role: 'acs:ram::123456789:role/aliyundevsdefaultrole',
region: 'cn-hangzhou',
vpcConfig: {},
Expand Down Expand Up @@ -297,8 +304,8 @@ describe('ArtModelService', () => {
],
timeout: 10, // 设置较小的超时值以便测试
},
storage: 'nas',
nasMountPoints: [{ mountDir: '/mnt/test' }],
ossMountPoints: [{ mountDir: '/mnt/oss' }],
role: 'acs:ram::123456789:role/aliyundevsdefaultrole',
region: 'cn-hangzhou',
vpcConfig: {},
Expand All @@ -312,30 +319,6 @@ describe('ArtModelService', () => {
},
} as any);

mockDevClient.fileManagerRsync.mockResolvedValue({
body: {
success: true,
data: {
taskID: 'task-123',
},
requestId: 'req-123',
},
} as any);

// 模拟超时情况 - 任务永远不会完成
mockDevClient.getFileManagerTask.mockResolvedValue({
body: {
data: {
finished: false,
startTime: Date.now() - 50 * 60 * 1000, // 50分钟前开始
progress: {
currentBytes: 512,
totalBytes: 1024,
},
},
},
} as any);

// 现在使用重试函数，我们需要模拟重试函数抛出超时错误
(
require('../../../src/subCommands/model/utils')
Expand Down Expand Up @@ -364,8 +347,8 @@ describe('ArtModelService', () => {
},
],
},
storage: 'nas',
nasMountPoints: [{ mountDir: '/mnt/test' }],
ossMountPoints: [{ mountDir: '/mnt/oss' }],
role: 'acs:ram::123456789:role/aliyundevsdefaultrole',
region: 'cn-hangzhou',
vpcConfig: {},
Expand All @@ -379,16 +362,6 @@ describe('ArtModelService', () => {
},
} as any);

mockDevClient.fileManagerRsync.mockResolvedValue({
body: {
success: true,
data: {
taskID: 'task-123',
},
requestId: 'req-123',
},
} as any);

// 现在使用重试函数，我们需要模拟重试函数抛出错误
(
require('../../../src/subCommands/model/utils')
Expand Down Expand Up @@ -442,8 +415,8 @@ describe('ArtModelService', () => {
],
conflictResolution: 'overwrite', // 这个值应该被环境变量覆盖
},
storage: 'nas',
nasMountPoints: [{ mountDir: '/mnt/test' }],
ossMountPoints: [{ mountDir: '/mnt/oss' }],
role: 'acs:ram::123456789:role/aliyundevsdefaultrole',
region: 'cn-hangzhou',
vpcConfig: {},
Expand All @@ -461,16 +434,6 @@ describe('ArtModelService', () => {
},
} as any);

mockDevClient.fileManagerRsync.mockResolvedValue({
body: {
success: true,
data: {
taskID: 'task-123',
},
requestId: 'req-123',
},
} as any);

// 现在使用重试函数，我们需要模拟重试函数成功执行
(
require('../../../src/subCommands/model/utils')
Expand Down Expand Up @@ -518,31 +481,16 @@ describe('ArtModelService', () => {
],
},
nasMountPoints: [{ mountDir: '/mnt/test' }],
ossMountPoints: [{ mountDir: '/mnt/oss' }],
role: 'acs:ram::123456789:role/aliyundevsdefaultrole',
region: 'cn-hangzhou',
vpcConfig: {},
};

mockDevClient.fileManagerRm.mockResolvedValue({
body: {
success: true,
data: {
taskID: 'task-123',
},
requestId: 'req-123',
},
} as any);

// 模拟 getFileManagerTask 返回成功状态
mockDevClient.getFileManagerTask.mockResolvedValue({
body: {
data: {
finished: true,
success: true,
},
requestId: 'req-456',
},
} as any);
// 现在使用重试函数，我们需要模拟重试函数成功执行
(
require('../../../src/subCommands/model/utils').retryFileManagerRm as jest.Mock
).mockResolvedValue({ success: true, fileName: 'file1.txt' });

// 添加 removeFileManagerTasks 的模拟
mockDevClient.removeFileManagerTasks.mockResolvedValue({
Expand All @@ -555,6 +503,9 @@ describe('ArtModelService', () => {

// 成功移除应该正常完成
await expect(artModelService.removeModel(name, params)).resolves.toBeUndefined();

// 验证重试函数被调用
expect(require('../../../src/subCommands/model/utils').retryFileManagerRm).toHaveBeenCalled();
});

it('should handle remove failure', async () => {
Expand All @@ -572,35 +523,66 @@ describe('ArtModelService', () => {
],
},
nasMountPoints: [{ mountDir: '/mnt/test' }],
ossMountPoints: [{ mountDir: '/mnt/oss' }],
role: 'acs:ram::123456789:role/aliyundevsdefaultrole',
region: 'cn-hangzhou',
vpcConfig: {},
};

mockDevClient.fileManagerRm.mockResolvedValue({
body: {
success: true,
data: {
taskID: 'task-123',
// 现在使用重试函数，我们需要模拟重试函数返回失败
(
require('../../../src/subCommands/model/utils').retryFileManagerRm as jest.Mock
).mockResolvedValue({ success: false, fileName: 'file1.txt', error: 'Remove failed' });

// 移除失败应该抛出异常
await expect(artModelService.removeModel(name, params)).rejects.toThrow();
});

it('should handle remove with upgrade files', async () => {
const name = 'test-project$test-env$test-function';
const params = {
modelConfig: {
target: {
uri: 'nas://auto',
},
files: [
{
source: { path: 'file1.txt' },
target: { path: 'file1.txt' },
},
],
upgrade: {
history: {
'v1.0': '/mnt/test/v1.0/file1.txt',
},
},
Comment on lines 554 to 558
Copy link

@coderabbitai coderabbitai bot Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix trailing comma to satisfy formatter.
CI reports a formatting failure here; add the trailing comma after the upgrade history entry.

🔧 Proposed fix 
           upgrade: {
             history: {
-              'v1.0': '/mnt/test/v1.0/file1.txt'
+              'v1.0': '/mnt/test/v1.0/file1.txt',
             },
           },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
upgrade: {
history: {
'v1.0': '/mnt/test/v1.0/file1.txt'
},
},
upgrade: {
history: {
'v1.0': '/mnt/test/v1.0/file1.txt',
},
},
🤖 Prompt for AI Agents 
In `@__tests__/ut/commands/artModelService_test.ts` around lines 554 - 558, The
object literal in the test contains upgrade -> history -> 'v1.0' without a
trailing comma; update the history entry in
__tests__/ut/commands/artModelService_test.ts so the 'v1.0':
'/mnt/test/v1.0/file1.txt' line ends with a comma (i.e., add a trailing comma to
the history entry) to satisfy the formatter; locate the upgrade and history
objects in the test (references: upgrade, history, 'v1.0') and add the comma.

requestId: 'req-123',
},
} as any);
nasMountPoints: [{ mountDir: '/mnt/test' }],
ossMountPoints: [{ mountDir: '/mnt/oss' }],
role: 'acs:ram::123456789:role/aliyundevsdefaultrole',
region: 'cn-hangzhou',
vpcConfig: {},
};

// 模拟 getFileManagerTask 返回错误状态
mockDevClient.getFileManagerTask.mockResolvedValue({
// 现在使用重试函数，我们需要模拟重试函数成功执行
(
require('../../../src/subCommands/model/utils').retryFileManagerRm as jest.Mock
).mockResolvedValue({ success: true, fileName: 'file1.txt' });

// 添加 removeFileManagerTasks 的模拟
mockDevClient.removeFileManagerTasks.mockResolvedValue({
body: {
data: {
finished: true,
success: false,
errorMessage: 'Remove failed',
},
requestId: 'req-456',
success: true,
data: {},
requestId: 'req-999',
},
} as any);

// 移除失败应该抛出异常
await expect(artModelService.removeModel(name, params)).rejects.toThrow();
// 成功移除应该正常完成
await expect(artModelService.removeModel(name, params)).resolves.toBeUndefined();

// 验证重试函数被调用
expect(require('../../../src/subCommands/model/utils').retryFileManagerRm).toHaveBeenCalled();
});
});

Expand Down Expand Up @@ -743,5 +725,16 @@ describe('ArtModelService', () => {

expect(result).toBe('file://mnt/custom/file1.txt'); // Should remove leading slash
});

it('should handle mountDir starting with slash', () => {
const result = (artModelService as any)._getDestinationPath(
'nas://auto',
{ target: { path: 'file1.txt' } },
[{ mountDir: '/mnt/nas' }], // mountDir starts with slash
[{ mountDir: '/mnt/oss' }],
);

expect(result).toBe('file://mnt/nas/file1.txt');
});
});
});
2 changes: 2 additions & 0 deletions __tests__/ut/commands/model_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,7 @@ describe('Model', () => {
conflictResolution: 'overwrite',
mode: 'once',
timeout: 30 * 1000,
upgrade: {},
},
region: 'cn-hangzhou',
functionName: 'test-function',
Expand Down Expand Up @@ -502,6 +503,7 @@ describe('Model', () => {
conflictResolution: 'skip',
mode: 'always',
timeout: 60 * 1000,
upgrade: {},
},
region: 'cn-hangzhou',
functionName: 'test-function',
Expand Down
Loading
Loading