Skip to content

Commit e8cdc4e

Browse files
committed
refactor: address review feedback — mechanism-agnostic log, stronger tests
- Reword partial-success log messages to remove SQS reference; add created/requested counts to log metadata - ScaleError test now uses numberOfRunners=3 to validate failedInstanceCount - Partial-success test now requests 3 runners and gets 1 back (truly partial)
1 parent b8c45d8 commit e8cdc4e

2 files changed

Lines changed: 12 additions & 8 deletions

File tree

lambdas/functions/control-plane/src/aws/runners.test.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -511,13 +511,15 @@ describe('create runner with errors', () => {
511511
it('test ScaleError with multiple error.', async () => {
512512
createFleetMockWithErrors(['UnfulfillableCapacity', 'MaxSpotInstanceCountExceeded', 'NotMappedError']);
513513

514-
await expect(createRunner(createRunnerConfig(defaultRunnerConfig))).rejects.toMatchObject({
514+
await expect(
515+
createRunner({ ...createRunnerConfig(defaultRunnerConfig), numberOfRunners: 3 }),
516+
).rejects.toMatchObject({
515517
name: 'ScaleError',
516-
failedInstanceCount: 1, // numberOfRunners when zero instances created
518+
failedInstanceCount: 3, // numberOfRunners when zero instances created
517519
});
518520
expect(mockEC2Client).toHaveReceivedCommandWith(
519521
CreateFleetCommand,
520-
expectedCreateFleetRequest(defaultExpectedFleetRequestValues),
522+
expectedCreateFleetRequest({ ...defaultExpectedFleetRequestValues, totalTargetCapacity: 3 }),
521523
);
522524
expect(mockSSMClient).not.toHaveReceivedCommand(PutParameterCommand);
523525
});
@@ -546,10 +548,12 @@ describe('create runner with errors', () => {
546548
it('returns partial instances on recognized scale error instead of throwing', async () => {
547549
createFleetMockWithErrors(['UnfulfillableCapacity'], ['i-partial']);
548550

549-
await expect(createRunner(createRunnerConfig(defaultRunnerConfig))).resolves.toEqual(['i-partial']);
551+
await expect(
552+
createRunner({ ...createRunnerConfig(defaultRunnerConfig), numberOfRunners: 3 }),
553+
).resolves.toEqual(['i-partial']);
550554
expect(mockEC2Client).toHaveReceivedCommandWith(
551555
CreateFleetCommand,
552-
expectedCreateFleetRequest(defaultExpectedFleetRequestValues),
556+
expectedCreateFleetRequest({ ...defaultExpectedFleetRequestValues, totalTargetCapacity: 3 }),
553557
);
554558
});
555559

lambdas/functions/control-plane/src/aws/runners.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -204,8 +204,8 @@ async function processFleetResult(
204204
if (instances.length > 0) {
205205
logger.warn(
206206
`Partial fleet success: ${instances.length}/${runnerParameters.numberOfRunners} instances created. ` +
207-
`Returning partial results; caller will retry the shortfall via SQS.`,
208-
{ data: fleet.Errors },
207+
`Returning partial results; unfulfilled requests remain for retry.`,
208+
{ data: fleet.Errors, created: instances.length, requested: runnerParameters.numberOfRunners },
209209
);
210210
return instances;
211211
}
@@ -218,7 +218,7 @@ async function processFleetResult(
218218
logger.warn(
219219
`Partial fleet success: ${instances.length}/${runnerParameters.numberOfRunners} instances created. ` +
220220
`Error not recognized as scaling error; returning partial results.`,
221-
{ data: fleet.Errors },
221+
{ data: fleet.Errors, created: instances.length, requested: runnerParameters.numberOfRunners },
222222
);
223223
return instances;
224224
}

0 commit comments

Comments
 (0)