Merge pull request #217 from check-run-reporter/node-count-and-index

node count and index

.github/workflows/push.yml

@@ -140,7 +140,7 @@ jobs:
           path: integrations/check-run-reporter-buildkite-plugin/bin
       # artifacts strip file permissions
       - run: chmod +x integrations/check-run-reporter-buildkite-plugin/bin/*
-      - run: ./integrations/check-run-reporter-buildkite-plugin/tests/bats/bin/bats ./integrations/check-run-reporter-buildkite-plugin/tests/post-command.bats
+      - run: ./integrations/check-run-reporter-buildkite-plugin/tests/bats/bin/bats ./integrations/check-run-reporter-buildkite-plugin/tests/*.bats
 
   release:
     needs:

README.md

@@ -71,12 +71,28 @@ has seen before will be evenly distributed while new test will be spread
 round-robin.
 
 ```sh
+JEST_JUNIT_OUTPUT_DIR='reports/junit' \
+JEST_JUNIT_ANCESTOR_SEPARATOR=' › ' \
+JEST_JUNIT_CLASSNAME='{classname}' \
+JEST_JUNIT_INCLUDE_CONSOLE_OUTPUT=true \
+JEST_JUNIT_OUTPUT_NAME='jest.xml' \
+JEST_JUNIT_SUITE_NAME='Some Label' \
+JEST_JUNIT_TITLE='{title}' \
+
 npx jest $(crr split \
   --label='Unit Tests' \
   --nodeCount=3 \
   --nodeIndex=1 \
   --tests='src/**/*.spec.ts' \
   --token=$CHECK_RUN_REPORTER_TOKEN)
+
+crr submit \
+  --label='Some Label' \
+  --nodeCount=3 \
+  --nodeIndex=1 \
+  --report='reports/junit/**/*.xml' \
+  --token='<your token>' \
+  --sha="$(git rev-parse HEAD)"
 ```
 
 ## Maintainers

integrations/action/README.md

@@ -44,7 +44,7 @@ jobs:
         steps:
             - uses: actions/checkout@master
             - uses: actions/setup-node@v1
-            with:
+              with:
                 node-version: '12.x'
             - run: npm ci
             - run: npm test
@@ -60,9 +60,9 @@ You can declare the action multiple times if you'd like to do separate
 submissions with different labels (for example, you want separate style report
 and test report submissions).
 
-Note the `if: ${{ always() }}`. By default, GitHub actions exit as soon as step
-fails. You'll need to tell GitHub to run even in event of failure to ensure your
-reports are submitted.
+Note the `if: ${{ always() }}`. By default, GitHub actions exit as soon as a
+step fails. You'll need to tell GitHub to run even in event of failure to ensure
+your reports are submitted.
 
 > See [action.yml](action.yml) for full configuration options.
 

integrations/action/action.yml

@@ -1,6 +1,6 @@
 name: 'Check Run Reporter'
 description: |
-  A GitHub action for uploading structured test reports to Check Run Reporter
+  A GitHub action for uploading structured test reports to check-run-reporter.com for reporting, stability analysic, and optimization.
 branding:
   icon: 'check'
   color: 'orange'
@@ -13,9 +13,11 @@ inputs:
       Label that should appear in the GitHub check. Defaults to the step's name.
     required: false
   nodeCount:
+    default: '1'
     description: The total number of nodes you intend to spin up
     required: false
   nodeIndex:
+    default: '0'
     description: This node's index.
     required: false
   report:

integrations/action/src/index.ts

@@ -1,3 +1,5 @@
+import assert from 'assert';
+
 import * as core from '@actions/core';
 import * as github from '@actions/github';
 import * as glob from '@actions/glob';
@@ -91,31 +93,33 @@ export async function findReports(): Promise<string[]> {
 
 interface DoSplitInput {
   readonly label: string;
+  readonly nodeCount: number;
+  readonly nodeIndex: number;
   readonly tests: string;
   readonly token: string;
 }
 
 /**
  * Wrapper around split to adapt it for github actions
  */
-async function doSplit({label, tests, token}: DoSplitInput, {client}: Context) {
-  const nodeCount = core.getInput('nodeCount');
-  const nodeIndex = core.getInput('nodeIndex');
-
-  if (!nodeCount) {
+async function doSplit(
+  {label, nodeCount, nodeIndex, tests, token}: DoSplitInput,
+  {client}: Context
+) {
+  if (Number.isNaN(Number(nodeCount))) {
     core.setFailed('Cannot split tests without specifying the nodeCount input');
   }
 
-  if (!nodeIndex) {
+  if (Number.isNaN(Number(nodeIndex))) {
     core.setFailed('Cannot split tests without specifying the nodeIndex input');
   }
 
   try {
     const {filenames} = await split(
       {
         label,
-        nodeCount: Number(nodeCount),
-        nodeIndex: Number(nodeIndex),
+        nodeCount,
+        nodeIndex,
         tests: [tests],
         token,
       },
@@ -153,6 +157,10 @@ async function main() {
 
   const hostname = core.getInput('hostname');
   const token = core.getInput('token');
+  const nodeCount = Number(core.getInput('nodeCount'));
+  assert(!Number.isNaN(nodeCount), 'nodeCount must be a number');
+  const nodeIndex = Number(core.getInput('nodeIndex'));
+  assert(!Number.isNaN(nodeIndex), 'nodeIndex must be a number');
 
   const client = makeClient({hostname});
 
@@ -161,6 +169,8 @@ async function main() {
     return await doSplit(
       {
         label,
+        nodeCount,
+        nodeIndex,
         tests,
         token,
       },
@@ -179,6 +189,8 @@ async function main() {
   await submit(
     {
       label,
+      nodeCount,
+      nodeIndex,
       report: files,
       root,
       sha,

integrations/check-run-reporter-buildkite-plugin/tests/post-command.bats

@@ -59,7 +59,6 @@ setup() {
     run "post-command"
 
     assert_failure
-    assert_output --partial 'code 401'
-    # The API isn't currently returning the following message.
-    # assert_output --partial 'Could not find repo token in Authorization header'
+    assert_output --partial 'code 403'
+    assert_output --partial 'Specified token does not identify a repository'
 }

integrations/check-run-reporter-buildkite-plugin/tests/pre-command-wrapper

@@ -5,8 +5,11 @@
 
 set -euo pipefail
 
-source pre-command
-env | awk '
-  /CHECK_RUN_REPORTER_TESTS_FOR_THIS_AGENT/ { found = 1 }
-  found { print $0}
-'
+# Reminder, this has to be sourced so that it can change the environment of the
+# current process so that the next line can work
+source pre-command &>/dev/null
+
+# I don't really know what this command does, but I couldn't quite get awk to do
+# what I wanted and after a few rounds with ChatGPT that did nothing, this
+# appears to work.
+env | sed -n '/^CHECK_RUN_REPORTER_TESTS_FOR_THIS_AGENT=/,/^.*$/p'

integrations/check-run-reporter-buildkite-plugin/tests/pre-command.bats

@@ -29,8 +29,8 @@ setup() {
 
     assert_success
 
-    assert_line --index 0 CHECK_RUN_REPORTER_TESTS_FOR_THIS_AGENT=logger.spec.ts
-    assert_line --index 1 'user.spec.ts'
+    assert_output 'CHECK_RUN_REPORTER_TESTS_FOR_THIS_AGENT=logger.spec.ts
+user.spec.ts'
 }
 
 @test "Without tests input" {

src/cli.ts

@@ -101,6 +101,14 @@ export function cli(argv: string[]) {
             description: 'Label that should appear in the GitHub check run.',
             type: 'string',
           },
+          nodeCount: {
+            default: 1,
+            type: 'number',
+          },
+          nodeIndex: {
+            default: 0,
+            type: 'number',
+          },
           report: {
             demandOption: true,
             description:

src/commands/submit.spec.ts

@@ -17,7 +17,17 @@ describe('submit()', () => {
 
   it('submits reports to check run reporter using multistep upload', async () => {
     nock('https://api.check-run-reporter.com')
-      .post('/api/v1/submissions/upload')
+      .post(
+        '/api/v1/submissions/upload',
+        JSON.stringify({
+          filenames: ['reports/junit/jest.xml'],
+          label: 'foo',
+          nodeCount: 1,
+          nodeIndex: 0,
+          root: '/',
+          sha: '40923a72ddf9eefef938355fa96246607c706f6c',
+        })
+      )
       .reply(201, {
         keys: [
           'ianwremmel/check-run-reporter/c6fb3d5423762aa2b3a8f63717ef6b320e5a1b5a/SOMEUUID-reports/junit/jest.xml',
@@ -29,36 +39,34 @@ describe('submit()', () => {
         },
       });
 
+    // Mock with and without a port due to potential environment differences
+    // that shouldn't actually matter.
     nock('https://example.com').put('/1').reply(200);
+    nock('https://example.com:443').put('/1').reply(200);
 
     nock('https://api.check-run-reporter.com')
-      .patch('/api/v1/submissions/upload')
+      .patch(
+        '/api/v1/submissions/upload',
+        JSON.stringify({
+          keys: [
+            'ianwremmel/check-run-reporter/c6fb3d5423762aa2b3a8f63717ef6b320e5a1b5a/SOMEUUID-reports/junit/jest.xml',
+          ],
+          label: 'foo',
+          nodeCount: 1,
+          nodeIndex: 0,
+          root: '/',
+          sha: '40923a72ddf9eefef938355fa96246607c706f6c',
+          signature:
+            '19c3cb9748107142317f4eb212b61d2be19a8c4b2aff9dc6117a7936b28313e5',
+        })
+      )
       .reply(202);
 
     await submit(
       {
         label: 'foo',
-        report: ['reports/junit/**/*.xml'],
-        root: '/',
-        sha: '40923a72ddf9eefef938355fa96246607c706f6c',
-        token: 'FAKE TOKEN',
-      },
-      makeTestContext()
-    );
-  });
-
-  it('submits reports to check run reporter, falling back to single step upload', async () => {
-    nock('https://api.check-run-reporter.com')
-      .post('/api/v1/submissions/upload')
-      .reply(404);
-
-    nock('https://api.check-run-reporter.com')
-      .post('/api/v1/submissions')
-      .reply(201);
-
-    await submit(
-      {
-        label: 'foo',
+        nodeCount: 1,
+        nodeIndex: 0,
         report: ['reports/junit/**/*.xml'],
         root: '/',
         sha: '40923a72ddf9eefef938355fa96246607c706f6c',

src/commands/submit.ts

@@ -3,12 +3,13 @@ import util from 'util';
 import axios from 'axios';
 
 import {Context, Optional} from '../lib/types';
-// eslint-disable-next-line import/no-deprecated
-import {multiStepUpload, singleStepUpload} from '../lib/upload';
+import {multiStepUpload} from '../lib/upload';
 import {getRequestId} from '../lib/axios';
 
 interface SubmitArgs {
   readonly label: Optional<string>;
+  readonly nodeCount: number;
+  readonly nodeIndex: number;
   readonly report: readonly string[];
   readonly root: string;
   readonly sha: string;
@@ -23,7 +24,7 @@ export async function submit(input: SubmitArgs, context: Context) {
 
   try {
     logger.group('Uploading report to Check Run Reporter');
-    await tryMultiStepUploadOrFallbackToSingle(input, context);
+    await multiStepUpload(input, context);
   } catch (err) {
     if (axios.isAxiosError(err)) {
       if (!err.response) {
@@ -48,33 +49,3 @@ export async function submit(input: SubmitArgs, context: Context) {
     logger.groupEnd();
   }
 }
-/**
- * Attempts to use multistep upload, but falls back to the legacy system if it
- * gets a 404. This _should_ make things future proof so it'll get more
- * efficient once the new version is released.
- */
-async function tryMultiStepUploadOrFallbackToSingle(
-  input: SubmitArgs,
-  context: Context
-) {
-  try {
-    return await multiStepUpload(input, context);
-  } catch (err) {
-    if (axios.isAxiosError(err)) {
-      // CI doesn't like safe-access here.
-      if (
-        (err.response && err.response.status === 404) ||
-        err.code === 'ECONNABORTED'
-      ) {
-        context.logger.info(
-          'Received 404 trying to get signed URLs. Assuming feature is notn released yet and falling back to single step upload',
-          {err}
-        );
-        // eslint-disable-next-line import/no-deprecated
-        return await singleStepUpload(input, context);
-      }
-    }
-
-    throw err;
-  }
-}

src/lib/logger.ts

@@ -3,7 +3,7 @@ import ci from 'ci-info';
 type LogLevel = 'debug' | 'info' | 'warn' | 'error';
 
 /**
- * Some CI services incldue utility syntax for doing novel things with log
+ * Some CI services include utility syntax for doing novel things with log
  * output. The Logger interfaces lets us do "the right thing" for those
  * services.
  */