Merge branch 'develop' of github.com:topcoder-platform/marathon-match-api-v6 into develop

Author: jmgasper

ECS_RUNNER_LIFETIME.md

@@ -195,6 +195,7 @@ For `PROVISIONAL` and `SYSTEM`, the runner posts progress to `POST /v6/marathon-
 - `timed_out: true` for SYSTEM timeout failures
 
 Treat `IN PROGRESS` summations as unavailable, even when they carry a placeholder score.
+Completed scoring may report `SUCCESS` with nonzero `failedTests` when individual testcases timed out or crashed; `FAILED` is reserved for explicit scorer or skipped-scoring failures.
 
 ## Stopping or killing runners
 

README.md

@@ -238,7 +238,7 @@ Auth model in code:
 | `POST` | `/v6/marathon-match/internal/system-score`     | `administrator` OR `update:marathon-match` |
 
 `POST /v6/marathon-match/internal/scoring-results` rejects callbacks whose `challengeId` does not map to an existing Marathon Match config.
-`POST /v6/marathon-match/internal/scoring-progress` stores runner progress in review summation metadata as `testProcess` (`provisional` or `system`), `testProgress` (`0` to `1`), and `testStatus` (`IN PROGRESS`, `SUCCESS`, or `FAILED`).
+`POST /v6/marathon-match/internal/scoring-progress` stores runner progress in review summation metadata as `testProcess` (`provisional` or `system`), `testProgress` (`0` to `1`), and `testStatus` (`IN PROGRESS`, `SUCCESS`, or `FAILED`). Completed scoring can still report `SUCCESS` when individual testcases timed out or crashed; those counts stay in `testProgressDetails.failedTests`.
 
 ## How to set up a challenge for marathon match scoring
 

docs/marathon-processor-specification-and-scoring-terminology.md

@@ -315,6 +315,8 @@ For Provisional and System scoring, the runner posts progress updates while test
 | `testProgressDetails.failedTests`    | testcase count with errors                |
 | `testProgressDetails.message`        | latest runner progress or failure message |
 
+Completed scoring may report `testStatus = SUCCESS` with nonzero `testProgressDetails.failedTests` when individual testcases timed out or crashed. `FAILED` is reserved for explicit scorer or skipped-scoring failures.
+
 Operators can also inspect ECS runner logs. Runner log mappings are stored for each launched scoring task and can be exposed through:
 
 ```text
@@ -376,7 +378,7 @@ For each testcase:
 - the best raw score receives `100`
 - all other valid scores receive `(lower score / higher score) * 100`
 
-The final relative score is the average of those per-testcase relative scores. If every testcase fails, the aggregate score is `-1`.
+The final relative score is the average of those per-testcase relative scores. If every testcase fails, the aggregate score is `-1` while completed test status remains `SUCCESS`.
 
 ### Score direction
 

docs/review-phase-scoring.md

@@ -66,7 +66,7 @@ sequenceDiagram
 
 After `ScoringResultService` writes the SYSTEM review summation, `completeSystemReviewIfNeeded` patches the originating review to `COMPLETED` and writes the final score back to Review API.
 
-While SYSTEM tests are running, the ECS runner updates the phase review summation metadata with `testProcess` (`provisional` or `system`), `testProgress` (`0` to `1`), and `testStatus` (`IN PROGRESS`, `SUCCESS`, or `FAILED`). These fields are returned by Review API under `reviewSummation.metadata` when metadata is requested. In-progress summations keep a neutral placeholder score and should be displayed as unavailable from `testStatus`; failed progress uses the failed-score sentinel.
+While SYSTEM tests are running, the ECS runner updates the phase review summation metadata with `testProcess` (`provisional` or `system`), `testProgress` (`0` to `1`), and `testStatus` (`IN PROGRESS`, `SUCCESS`, or `FAILED`). These fields are returned by Review API under `reviewSummation.metadata` when metadata is requested. In-progress summations keep a neutral placeholder score and should be displayed as unavailable from `testStatus`; explicit failed progress uses the failed-score sentinel. Completed scoring can report `testStatus = SUCCESS` with nonzero `testProgressDetails.failedTests` when individual testcases timed out or crashed.
 
 Each dispatched SYSTEM task also schedules a pg-boss timeout check using the config's `systemTestTimeout` value. The default is 24 hours (`86400000` ms). When the delayed check runs, the API describes the ECS task and checks the SYSTEM summation; if the task is still active and the summation is not complete, it stops the ECS task, writes a failed SYSTEM summation with score `-1`, and includes `metadata.timed_out = true`.
 

docs/submission-phase-scoring.md

@@ -99,7 +99,7 @@ Runner task logs are written to CloudWatch using the submission runner log strea
 
 `GET /v6/marathon-match/submissions/:submissionId/runner-logs`
 
-For `PROVISIONAL` and `SYSTEM` scoring, the runner writes progress to the phase review summation metadata through `POST /v6/marathon-match/internal/scoring-progress`. Review API exposes this as `reviewSummation.metadata.testProcess` (`provisional` or `system`), `reviewSummation.metadata.testProgress` (`0` to `1`), and `reviewSummation.metadata.testStatus` (`IN PROGRESS`, `SUCCESS`, or `FAILED`) when metadata is included in the response. In-progress summations keep a neutral placeholder score and must be rendered as unavailable based on `testStatus`; only `FAILED` progress uses the failed-score sentinel. SYSTEM timeout failures also include `reviewSummation.metadata.timed_out = true`.
+For `PROVISIONAL` and `SYSTEM` scoring, the runner writes progress to the phase review summation metadata through `POST /v6/marathon-match/internal/scoring-progress`. Review API exposes this as `reviewSummation.metadata.testProcess` (`provisional` or `system`), `reviewSummation.metadata.testProgress` (`0` to `1`), and `reviewSummation.metadata.testStatus` (`IN PROGRESS`, `SUCCESS`, or `FAILED`) when metadata is included in the response. In-progress summations keep a neutral placeholder score and must be rendered as unavailable based on `testStatus`; only explicit scorer or skipped-scoring failures use `FAILED` progress and the failed-score sentinel. Completed scoring with timed-out or crashed testcases keeps those counts in `testProgressDetails.failedTests` while reporting `testStatus = SUCCESS`. SYSTEM timeout failures also include `reviewSummation.metadata.timed_out = true`.
 
 For generic runner artifacts, only EXAMPLE scoring uploads the member-visible public artifact. Its public `output.txt` includes each testcase ordinal, actual seed, seed score, runtime, runner/tester errors, and submitted-solution stderr, while submitted-solution stdout and compilation diagnostics stay out of that file. The separate `compile_log.txt` file preserves compilation output. Every generic scoring phase uploads private internal artifacts: `reviews.json` includes `compilationOutput`, per-test score details with testcase ordinals and actual seeds, and top-level `testScores`; callback metadata keeps member-visible testcase ordinals only, compile and execution logs are preserved as `compile_log.txt`, `execution-{submissionId}.log`, and optional `error-{submissionId}.log`, and captured submitted-solution output is split into `stdout/{seed}.txt` and `stderr/{seed}.txt` files for each actual seed. PROVISIONAL and SYSTEM scoring do not publish competitor-visible artifacts.
 

ecs-runner/src/main/java/com/topcoder/runner/EcsRunnerMain.java

@@ -4727,7 +4727,7 @@ private static TesterExecutionResult runGenericMarathonTester(
                     testScores.size(),
                     numberOfTests,
                     failedTests,
-                    failedTests > 0 ? TEST_STATUS_FAILED : TEST_STATUS_IN_PROGRESS,
+                    TEST_STATUS_IN_PROGRESS,
                     "Completed test " + testCaseNumber + " of " + numberOfTests
                 );
             }

package.json

@@ -48,7 +48,7 @@
     "jsonwebtoken": "^9.0.2",
     "jwks-rsa": "^3.2.0",
     "lodash": "^4.17.21",
-    "multer": "^2.1.1",
+    "multer": "^2.2.0",
     "nanoid": "~5.1.2",
     "pg": "^8.16.3",
     "pg-boss": "^11.0.5",

pnpm-lock.yaml

@@ -339,6 +339,68 @@ describe('ScoringResultService', () => {
     );
   });
 
+  it('keeps completed scoring successful when all individual tests fail', async () => {
+    const { service, m2mService, prisma } = createService();
+    const payloadWithFailedSeed: ScoringResultCallbackPayload = {
+      ...basePayload,
+      score: -1,
+      metadata: {
+        numberOfTests: 2,
+        testScores: [
+          {
+            error: 'Timed out.',
+            score: -1,
+            testcase: '753388858',
+          },
+          {
+            error: 'Crashed.',
+            score: -1,
+            testcase: '753388859',
+          },
+        ],
+      },
+    };
+
+    prisma.marathonMatchConfig.findUnique.mockResolvedValue({
+      challengeId: basePayload.challengeId,
+      name: 'Blocks',
+      submissionApiUrl: 'https://api.topcoder-dev.com/v6',
+      relativeScoringEnabled: false,
+      scoreDirection: ScoreDirection.MAXIMIZE,
+    });
+    m2mService.getM2MToken.mockResolvedValue('m2m-token');
+
+    const createReviewSummationSpy = jest
+      .spyOn(service as any, 'createReviewSummation')
+      .mockResolvedValue(undefined);
+    jest
+      .spyOn(service as any, 'findExistingReviewSummations')
+      .mockResolvedValue([]);
+    jest
+      .spyOn(service as any, 'completeSystemReviewIfNeeded')
+      .mockResolvedValue(undefined);
+
+    await expect(
+      service.processScoringResult(payloadWithFailedSeed),
+    ).resolves.toBe(undefined);
+
+    expect(createReviewSummationSpy).toHaveBeenCalledWith(
+      'm2m-token',
+      expect.objectContaining({
+        aggregateScore: payloadWithFailedSeed.score,
+        isPassing: false,
+        metadata: expect.objectContaining({
+          testProgress: 1,
+          testStatus: ScoringTestStatus.Success,
+          testProgressDetails: expect.objectContaining({
+            failedTests: 2,
+            status: ScoringTestStatus.Success,
+          }),
+        }),
+      }),
+    );
+  });
+
   it('persists finite relative scores when the maximize best testcase score is zero', async () => {
     const { service, m2mService, prisma } = createService();
     const zeroScorePayload: ScoringResultCallbackPayload = {
@@ -1639,6 +1701,73 @@ describe('ScoringResultService', () => {
     expect(result.payload.reviewedDate).toBe(reviewedDate);
   });
 
+  it('keeps relative scoring status successful when all individual tests fail', () => {
+    const { service } = createService();
+    const buildRelativeReviewPayload = (
+      service as any
+    ).buildRelativeReviewPayload.bind(service) as (
+      reviewRecord: {
+        submissionId: string;
+        reviewObject: Record<string, unknown>;
+        metadata: Record<string, unknown>;
+        rawTestScores: Array<{
+          error?: string;
+          score: number;
+          testcase: string;
+        }>;
+      },
+      bestScores: Map<string, number>,
+      scoreDirection: ScoreDirection,
+      fallbackScorecardId: string | undefined,
+      testPhase: string,
+    ) => {
+      payload: {
+        aggregateScore: number;
+        metadata: {
+          testProgressDetails: {
+            failedTests: number;
+            status: ScoringTestStatus;
+          };
+          testStatus: ScoringTestStatus;
+        };
+      };
+    };
+
+    const result = buildRelativeReviewPayload(
+      {
+        submissionId: 'failed-testcase-submission',
+        reviewObject: {
+          id: 'review-summation-1',
+          isProvisional: true,
+        },
+        metadata: {
+          testScores: [
+            { error: 'Timed out.', score: -1, testcase: '1' },
+            { error: 'Crashed.', score: -1, testcase: '2' },
+          ],
+          testType: 'provisional',
+        },
+        rawTestScores: [
+          { error: 'Timed out.', score: -1, testcase: '1' },
+          { error: 'Crashed.', score: -1, testcase: '2' },
+        ],
+      },
+      new Map(),
+      ScoreDirection.MAXIMIZE,
+      undefined,
+      'provisional',
+    );
+
+    expect(result.payload.aggregateScore).toBe(-1);
+    expect(result.payload.metadata.testStatus).toBe(ScoringTestStatus.Success);
+    expect(result.payload.metadata.testProgressDetails).toEqual(
+      expect.objectContaining({
+        failedTests: 2,
+        status: ScoringTestStatus.Success,
+      }),
+    );
+  });
+
   it('marks timed-out system tests as failed with timed_out metadata', async () => {
     const { service } = createService();
     const processScoringResultSpy = jest

src/api/scoring-result/scoring-result.service.spec.ts

@@ -2183,9 +2183,7 @@ export class ScoringResultService {
         failedTests,
         progress: 1,
         status:
-          failedTests > 0
-            ? ScoringTestStatus.Failed
-            : ScoringTestStatus.Success,
+          totalTests > 0 ? ScoringTestStatus.Success : ScoringTestStatus.Failed,
         totalTests,
       },
     );
@@ -2687,11 +2685,11 @@ export class ScoringResultService {
     finalScore: number,
   ): Record<string, unknown> {
     const totalTests = this.resolveTotalTests(metadata);
-    const completedTests =
-      totalTests ?? this.countCompletedTestScores(metadata);
+    const completedTestScores = this.countCompletedTestScores(metadata);
+    const completedTests = totalTests ?? completedTestScores;
     const failedTests = this.countFailedTestScores(metadata);
     const status =
-      finalScore < 0 || failedTests > 0
+      finalScore < 0 && completedTestScores === 0
         ? ScoringTestStatus.Failed
         : ScoringTestStatus.Success;