From 4dca63d1e2348bd3ae5df587b73983cacd4f468f Mon Sep 17 00:00:00 2001 From: ianwremmel <1182361+ianwremmel@users.noreply.github.com> Date: Thu, 11 May 2023 20:11:42 -0700 Subject: [PATCH 1/5] ci: run all bats tests --- .github/workflows/push.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index 9ed3005..2a9076a 100644 --- a/.github/workflows/push.yml +++ b/.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: From c13a6b0dfd12e9bfc928a5edd4c615563f117350 Mon Sep 17 00:00:00 2001 From: ianwremmel <1182361+ianwremmel@users.noreply.github.com> Date: Thu, 11 May 2023 20:12:32 -0700 Subject: [PATCH 2/5] test: fix all buildkite tests --- .../tests/post-command.bats | 5 ++--- .../tests/pre-command-wrapper | 13 ++++++++----- .../tests/pre-command.bats | 4 ++-- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/integrations/check-run-reporter-buildkite-plugin/tests/post-command.bats b/integrations/check-run-reporter-buildkite-plugin/tests/post-command.bats index fa548da..39df7ad 100644 --- a/integrations/check-run-reporter-buildkite-plugin/tests/post-command.bats +++ b/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' } diff --git a/integrations/check-run-reporter-buildkite-plugin/tests/pre-command-wrapper b/integrations/check-run-reporter-buildkite-plugin/tests/pre-command-wrapper index 58042d9..ba549c6 100755 --- a/integrations/check-run-reporter-buildkite-plugin/tests/pre-command-wrapper +++ b/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' diff --git a/integrations/check-run-reporter-buildkite-plugin/tests/pre-command.bats b/integrations/check-run-reporter-buildkite-plugin/tests/pre-command.bats index 2f59352..f3035e1 100644 --- a/integrations/check-run-reporter-buildkite-plugin/tests/pre-command.bats +++ b/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" { From 0d570e6ea82da7f64b98b08fc571a4dc9d6aa2f9 Mon Sep 17 00:00:00 2001 From: ianwremmel <1182361+ianwremmel@users.noreply.github.com> Date: Thu, 11 May 2023 20:19:35 -0700 Subject: [PATCH 3/5] refactor: remove single-step-upload now that the backend has support multistep for some time --- src/commands/submit.ts | 35 ++-------------------------- src/lib/logger.ts | 2 +- src/lib/upload.ts | 52 ++---------------------------------------- 3 files changed, 5 insertions(+), 84 deletions(-) diff --git a/src/commands/submit.ts b/src/commands/submit.ts index 979e060..9bc35bd 100644 --- a/src/commands/submit.ts +++ b/src/commands/submit.ts @@ -3,8 +3,7 @@ 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 { @@ -23,7 +22,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 +47,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; - } -} diff --git a/src/lib/logger.ts b/src/lib/logger.ts index 24f7ddd..db1121c 100644 --- a/src/lib/logger.ts +++ b/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. */ diff --git a/src/lib/upload.ts b/src/lib/upload.ts index c760740..f300196 100644 --- a/src/lib/upload.ts +++ b/src/lib/upload.ts @@ -1,10 +1,7 @@ import fs from 'fs'; -import FormData from 'form-data'; +import {PATH_MULTI_STEP_UPLOAD} from '../constants'; -import {PATH_MULTI_STEP_UPLOAD, PATH_SINGLE_STEP_UPLOAD} from '../constants'; - -import {getRequestId} from './axios'; import {multiGlob} from './file'; import {Context, Optional} from './types'; @@ -17,51 +14,6 @@ interface UploadArgs { } type URLs = Record; -/** - * Uploads directly to Check Run Reporter. This is a legacy solution that no - * longer works for large submissions thanks to new backend architecture. It - * remains for compatibility reasons during the transition period, but multstep - * is the preferred method going forward. - * @deprecated use multiStepUpload instead - */ -export async function singleStepUpload( - {label, report, root, sha, token}: UploadArgs, - context: Context -) { - const {client, logger} = context; - - logger.info(`Label: ${label}`); - logger.info(`Root: ${root}`); - logger.info(`SHA: ${sha}`); - - const filenames = await multiGlob(report, context); - - const formData = new FormData(); - for (const filename of filenames) { - formData.append('report', fs.createReadStream(filename)); - } - - if (label) { - formData.append('label', label); - } - formData.append('root', root); - formData.append('sha', sha); - - const response = await client.post(PATH_SINGLE_STEP_UPLOAD, formData, { - auth: {password: token, username: 'token'}, - headers: { - ...formData.getHeaders(), - }, - maxContentLength: Infinity, - }); - - logger.info(`Request ID: ${getRequestId(response)}`); - logger.info(`Status: ${response.status}`); - logger.info(`StatusText: ${response.statusText}`); - logger.info(JSON.stringify(response.data, null, 2)); - - return response; -} /** * Orchestrates the multi-step upload process. @@ -77,7 +29,7 @@ export async function multiStepUpload(args: UploadArgs, context: Context) { logger.info(`Root: ${root}`); logger.info(`SHA: ${sha}`); - const filenames = await multiGlob(report, context); + const filenames = multiGlob(report, context); logger.group('Requesting signed urls'); const {keys, urls, signature} = await getSignedUploadUrls( args, From d52cbe6051009c90e198d47f67cae745ce542bb3 Mon Sep 17 00:00:00 2001 From: ianwremmel <1182361+ianwremmel@users.noreply.github.com> Date: Thu, 11 May 2023 20:51:23 -0700 Subject: [PATCH 4/5] fixup! refactor: remove single-step-upload now that the backend has support multistep for some time --- src/commands/submit.spec.ts | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/src/commands/submit.spec.ts b/src/commands/submit.spec.ts index 0266adb..4279509 100644 --- a/src/commands/submit.spec.ts +++ b/src/commands/submit.spec.ts @@ -46,25 +46,4 @@ describe('submit()', () => { 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', - report: ['reports/junit/**/*.xml'], - root: '/', - sha: '40923a72ddf9eefef938355fa96246607c706f6c', - token: 'FAKE TOKEN', - }, - makeTestContext() - ); - }); }); From 6d143264d65bf0f07b6b74fc37b0fc8b1c2169d8 Mon Sep 17 00:00:00 2001 From: ianwremmel <1182361+ianwremmel@users.noreply.github.com> Date: Thu, 11 May 2023 21:05:22 -0700 Subject: [PATCH 5/5] feat: add nodeCount and nodeIndex to all requests [Finishes #185127644] --- README.md | 16 ++++++++++++++++ integrations/action/README.md | 8 ++++---- integrations/action/action.yml | 4 +++- integrations/action/src/index.ts | 28 +++++++++++++++++++-------- src/cli.ts | 8 ++++++++ src/commands/submit.spec.ts | 33 ++++++++++++++++++++++++++++++-- src/commands/submit.ts | 2 ++ src/lib/upload.ts | 10 +++++++--- 8 files changed, 91 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 8f8d6c8..b90cd69 100644 --- a/README.md +++ b/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='' \ + --sha="$(git rev-parse HEAD)" ``` ## Maintainers diff --git a/integrations/action/README.md b/integrations/action/README.md index 7579ac4..7ba5866 100644 --- a/integrations/action/README.md +++ b/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. diff --git a/integrations/action/action.yml b/integrations/action/action.yml index aa00858..7e18a1c 100644 --- a/integrations/action/action.yml +++ b/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: diff --git a/integrations/action/src/index.ts b/integrations/action/src/index.ts index 18c4dea..8c2dcdf 100644 --- a/integrations/action/src/index.ts +++ b/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,6 +93,8 @@ export async function findReports(): Promise { interface DoSplitInput { readonly label: string; + readonly nodeCount: number; + readonly nodeIndex: number; readonly tests: string; readonly token: string; } @@ -98,15 +102,15 @@ interface DoSplitInput { /** * 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'); } @@ -114,8 +118,8 @@ async function doSplit({label, tests, token}: DoSplitInput, {client}: Context) { 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, diff --git a/src/cli.ts b/src/cli.ts index 762dd05..0f083e3 100644 --- a/src/cli.ts +++ b/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: diff --git a/src/commands/submit.spec.ts b/src/commands/submit.spec.ts index 4279509..b71d44f 100644 --- a/src/commands/submit.spec.ts +++ b/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,15 +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', + nodeCount: 1, + nodeIndex: 0, report: ['reports/junit/**/*.xml'], root: '/', sha: '40923a72ddf9eefef938355fa96246607c706f6c', diff --git a/src/commands/submit.ts b/src/commands/submit.ts index 9bc35bd..f5d21ca 100644 --- a/src/commands/submit.ts +++ b/src/commands/submit.ts @@ -8,6 +8,8 @@ import {getRequestId} from '../lib/axios'; interface SubmitArgs { readonly label: Optional; + readonly nodeCount: number; + readonly nodeIndex: number; readonly report: readonly string[]; readonly root: string; readonly sha: string; diff --git a/src/lib/upload.ts b/src/lib/upload.ts index f300196..490d625 100644 --- a/src/lib/upload.ts +++ b/src/lib/upload.ts @@ -7,6 +7,8 @@ import {Context, Optional} from './types'; interface UploadArgs { readonly label: Optional; + readonly nodeCount: number; + readonly nodeIndex: number; readonly report: readonly string[]; readonly root: string; readonly sha: string; @@ -53,11 +55,11 @@ export async function getSignedUploadUrls( filenames: readonly string[], {client}: Context ): Promise<{keys: string[]; signature: string; urls: Record}> { - const {label, root, sha, token} = args; + const {label, nodeCount, nodeIndex, root, sha, token} = args; const response = await client.post( PATH_MULTI_STEP_UPLOAD, - {filenames, label, root, sha}, + {filenames, label, nodeCount, nodeIndex, root, sha}, { auth: {password: token, username: 'token'}, maxContentLength: Infinity, @@ -94,13 +96,15 @@ export async function finishMultistepUpload( signature: string, {client}: Context ) { - const {label, root, sha, token} = args; + const {label, nodeCount, nodeIndex, root, sha, token} = args; const response = await client.patch( PATH_MULTI_STEP_UPLOAD, { keys, label, + nodeCount, + nodeIndex, root, sha, signature,