Skip to content

Commit a3e41b6

Browse files
fix: address comments
1 parent 6374b4f commit a3e41b6

10 files changed

Lines changed: 109 additions & 23 deletions

File tree

agent_docs/rules.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ Every new method must also have an integration test in `tests/integration/shared
2828
- Use `registerResource()` from `tests/integration/utils/cleanup.ts` for cleanup tracking
2929
- Use `generateRandomString()` from `tests/integration/utils/helpers.ts` for unique test data
3030
- Tests run in both `v0` and `v1` init modes via `describe.each(modes)`**only if the service is registered in both modes in `unified-setup.ts`**. New services that only support `v1` init should use `['v1']` only.
31+
- **Always `throw new Error()` when test preconditions are not met** — whether it's missing config (e.g., no `folderId`) or missing test data (e.g., no running jobs). Never use `console.warn()` + `return` to silently skip — silent skips hide unrunnable tests and make CI green when tests aren't actually exercised.
3132
- **NEVER** write redundant integration tests — each test must cover a distinct code path, error scenario, or response shape aspect.
3233
- **Include a transform validation test** for new methods with a transform pipeline. This test should verify: (a) transformed camelCase fields exist and have values (`job.createdTime`, `job.processName`), AND (b) original PascalCase API fields are absent (`(job as any).CreationTime` is `undefined`, `(job as any).ReleaseName` is `undefined`). This is a separate test from the basic "should retrieve by ID" test — it validates the SDK transform layer against the live API. Note: existing integration tests don't yet follow this pattern, but unit tests do (Assets, Queues, ChoiceSets). Extending it to integration tests catches mismatches between the Swagger spec assumptions and the live API response.
3334

docs/oauth-scopes.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ This page lists the specific OAuth scopes required in external app for each SDK
1616
| `getAll()` | `OR.Jobs` or `OR.Jobs.Read` |
1717
| `getById()` | `OR.Jobs` or `OR.Jobs.Read` |
1818
| `getOutput()` | `OR.Jobs` or `OR.Jobs.Read` |
19-
| `stop()` | `OR.Jobs` or `OR.Jobs.Write` |
19+
| `stop()` | `OR.Jobs` |
2020

2121
## Attachments
2222

src/core/http/api-client.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ export class ApiClient {
103103
throw ErrorFactory.createFromHttpStatus(response.status, errorInfo);
104104
}
105105

106-
if (response.status === 204) {
106+
if (response.status === 204 || response.headers.get('content-length') === '0') {
107107
return undefined as T;
108108
}
109109

src/models/orchestrator/jobs.constants.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
/** Maximum number of job keys to resolve in a single OData filter query (matches Python SDK) */
2+
export const JOB_KEY_RESOLUTION_CHUNK_SIZE = 50;
3+
14
/**
25
* Maps fields for Job entities to ensure consistent naming
36
* Semantic renames only — case conversion handled by pascalToCamelCaseKeys()

src/models/orchestrator/jobs.models.ts

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,7 @@ export interface JobServiceModel {
134134
/**
135135
* Stops one or more jobs by their UUID keys.
136136
*
137-
* Resolves the provided job UUID keys to integer IDs, then sends a stop request to the Orchestrator.
138-
* Keys are processed in chunks of 50 to avoid URL length limits. Throws if any keys cannot be resolved.
137+
* Sends a stop request for the specified jobs to the Orchestrator. Throws if any keys cannot be resolved.
139138
*
140139
* @param jobKeys - Array of job UUID keys to stop (e.g., from {@link JobGetResponse}.key)
141140
* @param folderId - The folder ID where the jobs reside (required)
@@ -192,6 +191,26 @@ export interface JobMethods {
192191
* ```
193192
*/
194193
getOutput(): Promise<Record<string, unknown> | null>;
194+
195+
/**
196+
* Stops this job.
197+
*
198+
* Sends a stop request for this job to the Orchestrator.
199+
*
200+
* @param options - Optional {@link JobStopOptions} including stop strategy (defaults to SoftStop)
201+
* @returns Promise resolving to an {@link OperationResponse}<{@link JobStopData}> with the resolved job ID
202+
*
203+
* @example
204+
* ```typescript
205+
* const allJobs = await jobs.getAll({ folderId: <folderId> });
206+
* const runningJob = allJobs.items.find(j => j.state === JobState.Running);
207+
*
208+
* if (runningJob) {
209+
* const result = await runningJob.stop();
210+
* }
211+
* ```
212+
*/
213+
stop(options?: JobStopOptions): Promise<OperationResponse<JobStopData>>;
195214
}
196215

197216
/**
@@ -208,6 +227,11 @@ function createJobMethods(jobData: RawJobGetResponse, service: JobServiceModel):
208227
if (!jobData.folderId) throw new Error('Job folderId is undefined');
209228
return service.getOutput(jobData.key, jobData.folderId);
210229
},
230+
async stop(options?: JobStopOptions): Promise<OperationResponse<JobStopData>> {
231+
if (!jobData.key) throw new Error('Job key is undefined');
232+
if (!jobData.folderId) throw new Error('Job folderId is undefined');
233+
return service.stop([jobData.key], jobData.folderId, options);
234+
},
211235
};
212236
}
213237

src/models/orchestrator/jobs.types.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,8 +177,8 @@ export interface JobGetByIdOptions extends BaseOptions {}
177177
export interface JobStopOptions {
178178
/**
179179
* The stop strategy to use.
180-
* - `SoftStop` — sends a cancellation request and waits for the job to finish gracefully
181-
* - `Kill` — forcefully terminates the job immediately
180+
* - `SoftStop` — requests graceful cancellation; the job completes its current activity before stopping
181+
* - `Kill` — requests immediate termination of the job
182182
* @default StopStrategy.SoftStop
183183
*/
184184
strategy?: StopStrategy;

src/services/orchestrator/jobs/jobs.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,13 @@ import { JobServiceModel, JobGetResponse, createJobWithMethods } from '../../../
44
import { addPrefixToKeys, pascalToCamelCaseKeys, transformData } from '../../../utils/transform';
55
import { JOB_ENDPOINTS } from '../../../utils/constants/endpoints';
66
import { ODATA_PAGINATION, ODATA_OFFSET_PARAMS, ODATA_PREFIX } from '../../../utils/constants/common';
7-
import { JobMap } from '../../../models/orchestrator/jobs.constants';
7+
import { JobMap, JOB_KEY_RESOLUTION_CHUNK_SIZE } from '../../../models/orchestrator/jobs.constants';
88
import { AttachmentService } from '../attachments/attachments';
99
import { ValidationError, ServerError } from '../../../core/errors';
1010
import { ErrorFactory } from '../../../core/errors/error-factory';
1111
import { errorResponseParser } from '../../../core/errors/parser';
1212
import { createHeaders } from '../../../utils/http/headers';
13-
import { FOLDER_ID, RESPONSE_TYPES } from '../../../utils/constants/headers';
13+
import { FOLDER_ID } from '../../../utils/constants/headers';
1414
import { PaginatedResponse, NonPaginatedResponse, HasPaginationOptions } from '../../../utils/pagination';
1515
import { PaginationHelpers } from '../../../utils/pagination/helpers';
1616
import { PaginationType } from '../../../utils/pagination/internal-types';
@@ -19,9 +19,6 @@ import type { IUiPath } from '../../../core/types';
1919
import { OperationResponse, CollectionResponse } from '../../../models/common/types';
2020
import { StopStrategy } from '../../../models/orchestrator/processes.types';
2121

22-
/** Maximum number of job keys to resolve in a single OData filter query */
23-
const JOB_KEY_RESOLUTION_CHUNK_SIZE = 50;
24-
2522
/**
2623
* Service for interacting with UiPath Orchestrator Jobs API
2724
*/
@@ -219,8 +216,7 @@ export class JobService extends FolderScopedService implements JobServiceModel {
219216
/**
220217
* Stops one or more jobs by their UUID keys.
221218
*
222-
* Resolves the provided job UUID keys to integer IDs, then sends a stop request to the Orchestrator.
223-
* Keys are processed in chunks of 50 to avoid URL length limits. Throws if any keys cannot be resolved.
219+
* Sends a stop request for the specified jobs to the Orchestrator. Throws if any keys cannot be resolved.
224220
*
225221
* @param jobKeys - Array of job UUID keys to stop (e.g., from {@link JobGetResponse}.key)
226222
* @param folderId - The folder ID where the jobs reside (required)
@@ -255,6 +251,10 @@ export class JobService extends FolderScopedService implements JobServiceModel {
255251
return { success: true, data: { jobIds: [] } };
256252
}
257253

254+
if (!folderId) {
255+
throw new ValidationError({ message: 'folderId is required for stop' });
256+
}
257+
258258
const headers = createHeaders({ [FOLDER_ID]: folderId });
259259
const strategy = options?.strategy ?? StopStrategy.SoftStop;
260260

@@ -341,7 +341,7 @@ export class JobService extends FolderScopedService implements JobServiceModel {
341341
throw new ValidationError({ message: `Jobs not found for keys: ${missingKeys.join(', ')}` });
342342
}
343343

344-
return jobKeys.map((key) => keyToIdMap.get(key)!);
344+
return uniqueKeys.map((key) => keyToIdMap.get(key)!);
345345
}
346346

347347
/**
@@ -355,7 +355,7 @@ export class JobService extends FolderScopedService implements JobServiceModel {
355355
await this.post(
356356
JOB_ENDPOINTS.STOP,
357357
{ jobIds, strategy },
358-
{ headers, responseType: RESPONSE_TYPES.TEXT }
358+
{ headers }
359359
);
360360
}
361361
}

tests/integration/shared/orchestrator/jobs.integration.test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,8 +204,7 @@ describe.each(modes)('Orchestrator Jobs - Integration Tests [%s]', (mode) => {
204204
});
205205

206206
if (runningJobs.items.length === 0) {
207-
console.warn('No running jobs available to test stop. Skipping.');
208-
return;
207+
throw new Error('No running jobs available in the test environment to test stop.');
209208
}
210209

211210
const jobKey = runningJobs.items[0].key;

tests/unit/models/orchestrator/jobs.test.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
import { createBasicJob } from '../../../utils/mocks/jobs';
77
import { JOB_TEST_CONSTANTS } from '../../../utils/constants/jobs';
88
import { TEST_CONSTANTS } from '../../../utils/constants/common';
9+
import { StopStrategy } from '../../../../src/models/orchestrator/processes.types';
910

1011
// ===== TEST SUITE =====
1112
describe('Job Models', () => {
@@ -16,6 +17,7 @@ describe('Job Models', () => {
1617
getAll: vi.fn(),
1718
getById: vi.fn(),
1819
getOutput: vi.fn(),
20+
stop: vi.fn(),
1921
} as any;
2022
});
2123

@@ -64,5 +66,54 @@ describe('Job Models', () => {
6466
await expect(job.getOutput()).rejects.toThrow('Job folderId is undefined');
6567
});
6668
});
69+
70+
describe('job.stop()', () => {
71+
it('should call service.stop with the job key and folderId', async () => {
72+
const mockJobData = createBasicJob();
73+
const job = createJobWithMethods(mockJobData, mockService);
74+
75+
const mockResult = { success: true, data: { jobIds: [JOB_TEST_CONSTANTS.JOB_ID] } };
76+
vi.mocked(mockService.stop).mockResolvedValue(mockResult);
77+
78+
const result = await job.stop();
79+
80+
expect(mockService.stop).toHaveBeenCalledWith(
81+
[JOB_TEST_CONSTANTS.JOB_KEY],
82+
TEST_CONSTANTS.FOLDER_ID,
83+
undefined
84+
);
85+
expect(result).toEqual(mockResult);
86+
});
87+
88+
it('should pass options to service.stop', async () => {
89+
const mockJobData = createBasicJob();
90+
const job = createJobWithMethods(mockJobData, mockService);
91+
92+
const mockResult = { success: true, data: { jobIds: [JOB_TEST_CONSTANTS.JOB_ID] } };
93+
vi.mocked(mockService.stop).mockResolvedValue(mockResult);
94+
95+
await job.stop({ strategy: StopStrategy.Kill });
96+
97+
expect(mockService.stop).toHaveBeenCalledWith(
98+
[JOB_TEST_CONSTANTS.JOB_KEY],
99+
TEST_CONSTANTS.FOLDER_ID,
100+
{ strategy: StopStrategy.Kill }
101+
);
102+
});
103+
104+
it('should throw when job key is undefined', async () => {
105+
const mockJobData = createBasicJob({ key: '' as any });
106+
const job = createJobWithMethods(mockJobData, mockService);
107+
108+
await expect(job.stop()).rejects.toThrow('Job key is undefined');
109+
});
110+
111+
it('should throw when job folderId is undefined', async () => {
112+
const mockJobData = createBasicJob({ folderId: undefined as any });
113+
const job = createJobWithMethods(mockJobData, mockService);
114+
115+
await expect(job.stop()).rejects.toThrow('Job folderId is undefined');
116+
});
117+
});
67118
});
68119
});

tests/unit/services/orchestrator/jobs.test.ts

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,8 @@ describe('JobService Unit Tests', () => {
174174

175175
expect(transformed.getOutput).toBeDefined();
176176
expect(typeof transformed.getOutput).toBe('function');
177+
expect(transformed.stop).toBeDefined();
178+
expect(typeof transformed.stop).toBe('function');
177179
});
178180
});
179181

@@ -216,14 +218,16 @@ describe('JobService Unit Tests', () => {
216218
expect((result as any).LastModificationTime).toBeUndefined();
217219
});
218220

219-
it('should attach getOutput method to result', async () => {
221+
it('should attach bound methods to result', async () => {
220222
const mockRawJob = createMockRawJob();
221223
mockApiClient.get.mockResolvedValueOnce(mockRawJob);
222224

223225
const result = await jobService.getById(JOB_TEST_CONSTANTS.JOB_KEY, TEST_CONSTANTS.FOLDER_ID);
224226

225227
expect(result.getOutput).toBeDefined();
226228
expect(typeof result.getOutput).toBe('function');
229+
expect(result.stop).toBeDefined();
230+
expect(typeof result.stop).toBe('function');
227231
});
228232

229233
it('should pass expand and select options with OData prefix', async () => {
@@ -583,7 +587,7 @@ describe('JobService Unit Tests', () => {
583587
).rejects.toThrow(JOB_TEST_CONSTANTS.ERROR_JOBS_NOT_FOUND_FOR_KEYS);
584588
});
585589

586-
it('should deduplicate job keys for resolution but preserve original order in result', async () => {
590+
it('should deduplicate job keys for resolution and return unique IDs', async () => {
587591
mockApiClient.get.mockResolvedValueOnce({
588592
value: [{ Key: JOB_TEST_CONSTANTS.JOB_KEY, Id: JOB_TEST_CONSTANTS.JOB_ID }],
589593
});
@@ -594,10 +598,8 @@ describe('JobService Unit Tests', () => {
594598
TEST_CONSTANTS.FOLDER_ID
595599
);
596600

597-
expect(result.data.jobIds).toEqual([
598-
JOB_TEST_CONSTANTS.JOB_ID,
599-
JOB_TEST_CONSTANTS.JOB_ID,
600-
]);
601+
// Duplicate keys are deduplicated — only one ID returned
602+
expect(result.data.jobIds).toEqual([JOB_TEST_CONSTANTS.JOB_ID]);
601603

602604
// Only one resolution call with deduplicated keys
603605
const filterArg = mockApiClient.get.mock.calls[0][1].params.$filter;
@@ -632,6 +634,12 @@ describe('JobService Unit Tests', () => {
632634
expect(result.data.jobIds).toEqual(ids);
633635
});
634636

637+
it('should throw validation error when folderId is missing', async () => {
638+
await expect(
639+
jobService.stop([JOB_TEST_CONSTANTS.JOB_KEY], 0)
640+
).rejects.toThrow('folderId is required for stop');
641+
});
642+
635643
it('should propagate resolution API errors', async () => {
636644
const error = createMockError(JOB_TEST_CONSTANTS.ERROR_JOB_NOT_FOUND);
637645
mockApiClient.get.mockRejectedValueOnce(error);

0 commit comments

Comments
 (0)