From 806be26275e9f355e2644b1e8e22e1a546714375 Mon Sep 17 00:00:00 2001 From: Martijn Pieters Date: Thu, 30 Mar 2023 14:22:51 +0100 Subject: [PATCH 1/2] Clean and spruce up the integration tests a bit - Use test-specific step ids - Remove unused input parameters - Provide clear output on what passed or failed, including a step summary for each passed test. --- .github/workflows/integration.yml | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index db4ac388..7304745a 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -8,6 +8,7 @@ on: jobs: test-return: + name: 'Integration test: return' runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 @@ -18,26 +19,34 @@ jobs: result-encoding: string input-value: output - run: | - if [[ "${{steps.output-set.outputs.result}}" != "output" ]]; then + echo "- Validating output is produced" + expected="output" + if [[ "${{steps.output-set.outputs.result}}" != "$expected" ]]; then + echo $'::error::\u274C' "Expected '$expected', got ${{steps.output-set.outputs.result}}" exit 1 fi + echo $'\u2705 Test passed' | tee -a $GITHUB_STEP_SUMMARY test-relative-require: + name: 'Integration test: relative-path require' runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 - - id: output-set + - id: relative-require uses: ./ with: script: return require('./package.json').name result-encoding: string - input-value: output - run: | - if [[ "${{steps.output-set.outputs.result}}" != "github-script" ]]; then + echo "- Validating relative require output" + if [[ "${{steps.relative-require.outputs.result}}" != "github-script" ]]; then + echo $'::error::\u274C' "Expected '$expected', got ${{steps.relative-require.outputs.result}}" exit 1 fi + echo $'\u2705 Test passed' | tee -a $GITHUB_STEP_SUMMARY test-npm-require: + name: 'Integration test: npm package require' runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 @@ -47,13 +56,16 @@ jobs: key: ${{runner.os}}-npm-${{hashFiles('**/package-lock.json')}} restore-keys: ${{runner.os}}-npm- - run: npm ci - - id: output-set + - id: npm-require uses: ./ with: script: return require('@actions/core/package.json').name result-encoding: string - input-value: output - run: | - if [[ "${{steps.output-set.outputs.result}}" != "@actions/core" ]]; then + echo "- Validating npm require output" + expected="@actions/core" + if [[ "${{steps.npm-require.outputs.result}}" != "$expected" ]]; then + echo $'::error::\u274C' "Expected '$expected', got ${{steps.npm-require.outputs.result}}" exit 1 fi + echo $'\u2705 Test passed' | tee -a $GITHUB_STEP_SUMMARY From 8d9f8fc050d038eac46e34e317ca8dc9b3a83e71 Mon Sep 17 00:00:00 2001 From: Martijn Pieters Date: Thu, 30 Mar 2023 14:53:55 +0100 Subject: [PATCH 2/2] Fix null handling, covered by integration tests `core.getInput()` always returns a string, so testing for 'not null' is always true. This then leads to previews set to an array with a single empty string, breaking accept-header output. Updated eslint rules should help avoid this issue in future, and new integration tests verify that the github client configuration now reflects the intended configuration options. --- .eslintrc.yml | 3 + .github/workflows/integration.yml | 180 ++++++++++++++++++++++++++++++ dist/index.js | 28 ++--- src/main.ts | 19 ++-- src/retry-options.ts | 2 +- tsconfig.eslint.json | 4 + 6 files changed, 210 insertions(+), 26 deletions(-) create mode 100644 tsconfig.eslint.json diff --git a/.eslintrc.yml b/.eslintrc.yml index 66131d37..746a83d2 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -6,8 +6,11 @@ extends: - plugin:@typescript-eslint/eslint-recommended - plugin:@typescript-eslint/recommended - prettier/@typescript-eslint +parserOptions: + project: ['tsconfig.eslint.json'] rules: # '@typescript-eslint/explicit-function-return-type': 0 '@typescript-eslint/no-use-before-define': - 2 - functions: false + '@typescript-eslint/no-unnecessary-condition': error diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 7304745a..9a8b67a5 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -69,3 +69,183 @@ jobs: exit 1 fi echo $'\u2705 Test passed' | tee -a $GITHUB_STEP_SUMMARY + + test-previews: + name: 'Integration test: previews option' + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - uses: actions/cache@v3 + with: + path: ~/.npm + key: ${{runner.os}}-npm-${{hashFiles('**/package-lock.json')}} + restore-keys: ${{runner.os}}-npm- + - run: npm ci + - id: previews-default + name: Default previews not set + uses: ./ + with: + script: | + const endpoint = github.request.endpoint + return endpoint({}).headers.accept + result-encoding: string + - id: previews-set-single + name: Previews set to a single value + uses: ./ + with: + previews: foo + script: | + const endpoint = github.request.endpoint + return endpoint({}).headers.accept + result-encoding: string + - id: previews-set-multiple + name: Previews set to comma-separated list + uses: ./ + with: + previews: foo,bar,baz + script: | + const endpoint = github.request.endpoint + return endpoint({}).headers.accept + result-encoding: string + - run: | + echo "- Validating previews default" + expected="application/vnd.github.v3+json" + if [[ "${{steps.previews-default.outputs.result}}" != $expected ]]; then + echo $'::error::\u274C' "Expected '$expected', got ${{steps.previews-default.outputs.result}}" + exit 1 + fi + echo "- Validating previews set to a single value" + expected="application/vnd.github.foo-preview+json" + if [[ "${{steps.previews-set-single.outputs.result}}" != $expected ]]; then + echo $'::error::\u274C' "Expected '$expected', got ${{steps.previews-set-single.outputs.result}}" + exit 1 + fi + echo "- Validating previews set to multiple values" + expected="application/vnd.github.foo-preview+json,application/vnd.github.bar-preview+json,application/vnd.github.baz-preview+json" + if [[ "${{steps.previews-set-multiple.outputs.result}}" != $expected ]]; then + echo $'::error::\u274C' "Expected '$expected', got ${{steps.previews-set-multiple.outputs.result}}" + exit 1 + fi + echo $'\u2705 Test passed' | tee -a $GITHUB_STEP_SUMMARY + + test-user-agent: + name: 'Integration test: user-agent option' + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - uses: actions/cache@v3 + with: + path: ~/.npm + key: ${{runner.os}}-npm-${{hashFiles('**/package-lock.json')}} + restore-keys: ${{runner.os}}-npm- + - run: npm ci + - id: user-agent-default + name: Default user-agent not set + uses: ./ + with: + script: | + const endpoint = github.request.endpoint + return endpoint({}).headers['user-agent'] + result-encoding: string + - id: user-agent-set + name: User-agent set + uses: ./ + with: + user-agent: foobar + script: | + const endpoint = github.request.endpoint + return endpoint({}).headers['user-agent'] + result-encoding: string + - id: user-agent-empty + name: User-agent set to an empty string + uses: ./ + with: + user-agent: '' + script: | + const endpoint = github.request.endpoint + return endpoint({}).headers['user-agent'] + result-encoding: string + - run: | + echo "- Validating user-agent default" + expected="actions/github-script octokit-core.js/" + if [[ "${{steps.user-agent-default.outputs.result}}" != "$expected"* ]]; then + echo $'::error::\u274C' "Expected user-agent to start with '$expected', got ${{steps.user-agent-default.outputs.result}}" + exit 1 + fi + echo "- Validating user-agent set to a value" + expected="foobar octokit-core.js/" + if [[ "${{steps.user-agent-set.outputs.result}}" != "$expected"* ]]; then + echo $'::error::\u274C' "Expected user-agent to start with '$expected', got ${{steps.user-agent-set.outputs.result}}" + exit 1 + fi + echo "- Validating user-agent set to an empty string" + expected="octokit-core.js/" + if [[ "${{steps.user-agent-empty.outputs.result}}" != "$expected"* ]]; then + echo $'::error::\u274C' "Expected user-agent to start with '$expected', got ${{steps.user-agent-empty.outputs.result}}" + exit 1 + fi + echo $'\u2705 Test passed' | tee -a $GITHUB_STEP_SUMMARY + + test-debug: + name: 'Integration test: debug option' + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - uses: actions/cache@v3 + with: + path: ~/.npm + key: ${{runner.os}}-npm-${{hashFiles('**/package-lock.json')}} + restore-keys: ${{runner.os}}-npm- + - run: npm ci + - id: debug-default + name: Default debug not set + uses: ./ + with: + script: | + const log = github.log + return { + debug: log.debug === console.debug, + info: log.info === console.info + } + - id: debug-true + name: Debug set to true + uses: ./ + with: + debug: true + script: | + const log = github.log + return { + debug: log.debug === console.debug, + info: log.info === console.info + } + - id: debug-false + name: Debug set to false + uses: ./ + with: + debug: false + script: | + const log = github.log + return { + debug: log.debug === console.debug, + info: log.info === console.info + } + - run: | + echo "- Validating debug default" + expected='{debug:false,info:false}' + if [[ "${{steps.debug-default.outputs.result}}" != "$expected" ]]; then + echo $'::error::\u274C' "Expected '$expected', got ${{steps.debug-default.outputs.result}}" + exit 1 + fi + echo "- Validating debug set to true" + expected='{debug:true,info:true}' + if [[ "${{steps.debug-true.outputs.result}}" != "$expected" ]]; then + echo $'::error::\u274C' "Expected '$expected', got ${{steps.debug-true.outputs.result}}" + exit 1 + fi + echo "- Validating debug set to false" + expected='{debug:false,info:false}' + if [[ "${{steps.debug-false.outputs.result}}" != "$expected" ]]; then + echo $'::error::\u274C' "Expected '$expected', got ${{steps.debug-false.outputs.result}}" + exit 1 + fi + echo $'\u2705 Test passed' | tee -a $GITHUB_STEP_SUMMARY diff --git a/dist/index.js b/dist/index.js index 809855bf..5eb9aacf 100644 --- a/dist/index.js +++ b/dist/index.js @@ -15134,6 +15134,9 @@ var io = __nccwpck_require__(7436); var dist_node = __nccwpck_require__(8883); // EXTERNAL MODULE: ./node_modules/@octokit/plugin-retry/dist-node/index.js var plugin_retry_dist_node = __nccwpck_require__(6298); +// EXTERNAL MODULE: ./node_modules/node-fetch/lib/index.js +var lib = __nccwpck_require__(467); +var lib_default = /*#__PURE__*/__nccwpck_require__.n(lib); ;// CONCATENATED MODULE: ./src/async-function.ts const AsyncFunction = Object.getPrototypeOf(async () => null).constructor; function callAsyncFunction(args, source) { @@ -15161,7 +15164,7 @@ function getRetryOptions(retries, exemptStatusCodes, defaultOptions) { ...defaultOptions.request, retries }; - core.debug(`GitHub client configured with: (retries: ${requestOptions.retries}, retry-exempt-status-code: ${(_a = retryOptions === null || retryOptions === void 0 ? void 0 : retryOptions.doNotRetry) !== null && _a !== void 0 ? _a : 'octokit default: [400, 401, 403, 404, 422]'})`); + core.debug(`GitHub client configured with: (retries: ${requestOptions.retries}, retry-exempt-status-code: ${(_a = retryOptions.doNotRetry) !== null && _a !== void 0 ? _a : 'octokit default: [400, 401, 403, 404, 422]'})`); return [retryOptions, requestOptions]; } function parseNumberArray(listString) { @@ -15197,9 +15200,6 @@ const wrapRequire = new Proxy(require, { } }); -// EXTERNAL MODULE: ./node_modules/node-fetch/lib/index.js -var lib = __nccwpck_require__(467); -var lib_default = /*#__PURE__*/__nccwpck_require__.n(lib); ;// CONCATENATED MODULE: ./src/main.ts @@ -15217,23 +15217,19 @@ process.on('unhandledRejection', handleError); main().catch(handleError); async function main() { const token = core.getInput('github-token', { required: true }); - const debug = core.getInput('debug'); + const debug = core.getBooleanInput('debug'); const userAgent = core.getInput('user-agent'); const previews = core.getInput('previews'); const retries = parseInt(core.getInput('retries')); const exemptStatusCodes = parseNumberArray(core.getInput('retry-exempt-status-codes')); const [retryOpts, requestOpts] = getRetryOptions(retries, exemptStatusCodes, utils.defaults); - const opts = {}; - if (debug === 'true') - opts.log = console; - if (userAgent != null) - opts.userAgent = userAgent; - if (previews != null) - opts.previews = previews.split(','); - if (retryOpts) - opts.retry = retryOpts; - if (requestOpts) - opts.request = requestOpts; + const opts = { + log: debug ? console : undefined, + userAgent: userAgent || undefined, + previews: previews ? previews.split(',') : undefined, + retry: retryOpts, + request: requestOpts + }; const github = (0,lib_github.getOctokit)(token, opts, plugin_retry_dist_node/* retry */.XD, dist_node/* requestLog */.g); const script = core.getInput('script', { required: true }); // Using property/value shorthand on `require` (e.g. `{require}`) causes compilation errors. diff --git a/src/main.ts b/src/main.ts index e86b4816..658cee06 100644 --- a/src/main.ts +++ b/src/main.ts @@ -7,10 +7,10 @@ import * as io from '@actions/io' import {requestLog} from '@octokit/plugin-request-log' import {retry} from '@octokit/plugin-retry' import {RequestRequestOptions} from '@octokit/types' +import fetch from 'node-fetch' import {callAsyncFunction} from './async-function' -import {getRetryOptions, parseNumberArray, RetryOptions} from './retry-options' +import {RetryOptions, getRetryOptions, parseNumberArray} from './retry-options' import {wrapRequire} from './wrap-require' -import fetch from 'node-fetch' process.on('unhandledRejection', handleError) main().catch(handleError) @@ -25,7 +25,7 @@ type Options = { async function main(): Promise { const token = core.getInput('github-token', {required: true}) - const debug = core.getInput('debug') + const debug = core.getBooleanInput('debug') const userAgent = core.getInput('user-agent') const previews = core.getInput('previews') const retries = parseInt(core.getInput('retries')) @@ -38,12 +38,13 @@ async function main(): Promise { defaultGitHubOptions ) - const opts: Options = {} - if (debug === 'true') opts.log = console - if (userAgent != null) opts.userAgent = userAgent - if (previews != null) opts.previews = previews.split(',') - if (retryOpts) opts.retry = retryOpts - if (requestOpts) opts.request = requestOpts + const opts: Options = { + log: debug ? console : undefined, + userAgent: userAgent || undefined, + previews: previews ? previews.split(',') : undefined, + retry: retryOpts, + request: requestOpts + } const github = getOctokit(token, opts, retry, requestLog) const script = core.getInput('script', {required: true}) diff --git a/src/retry-options.ts b/src/retry-options.ts index 6602d13e..683beaad 100644 --- a/src/retry-options.ts +++ b/src/retry-options.ts @@ -36,7 +36,7 @@ export function getRetryOptions( `GitHub client configured with: (retries: ${ requestOptions.retries }, retry-exempt-status-code: ${ - retryOptions?.doNotRetry ?? 'octokit default: [400, 401, 403, 404, 422]' + retryOptions.doNotRetry ?? 'octokit default: [400, 401, 403, 404, 422]' })` ) diff --git a/tsconfig.eslint.json b/tsconfig.eslint.json new file mode 100644 index 00000000..21c332eb --- /dev/null +++ b/tsconfig.eslint.json @@ -0,0 +1,4 @@ +{ + "extends": "./tsconfig.json", + "exclude": [] +} \ No newline at end of file