10000 Fix null handling, covered by integration tests · dekimsey/github-script@8d9f8fc · GitHub
[go: up one dir, main page]
Skip to content

Commit 8d9f8fc

Browse files
mjpietersmjpieters
committed
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.
1 parent 806be26 commit 8d9f8fc

File tree

6 files changed

+210
-26
lines changed

6 files changed

+210
-26
lines changed

‎.eslintrc.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,11 @@ extends:
66
- plugin:@typescript-eslint/eslint-recommended
77
- plugin:@typescript-eslint/recommended
88
- prettier/@typescript-eslint
9+
parserOptions:
10+
project: ['tsconfig.eslint.json']
911
rules:
1012
# '@typescript-eslint/explicit-function-return-type': 0
1113
'@typescript-eslint/no-use-before-define':
1214
- 2
1315
- functions: false
16+
'@typescript-eslint/no-unnecessary-condition': error

‎.github/workflows/integration.yml

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,3 +69,183 @@ jobs:
6969
exit 1
7070
fi
7171
echo $'\u2705 Test passed' | tee -a $GITHUB_STEP_SUMMARY
72+
73+
test-previews:
74+
name: 'Integration test: previews option'
75+
runs-on: ubuntu-latest
76+
steps:
77+
- uses: actions/checkout@v3
78+
- uses: actions/cache@v3
79+
with:
80+
path: ~/.npm
81+
key: ${{runner.os}}-npm-${{hashFiles('**/package-lock.json')}}
82+
restore-keys: ${{runner.os}}-npm-
83+
- run: npm ci
84+
- id: previews-default
85+
name: Default previews not set
86+
uses: ./
87+
with:
88+
script: |
89+
const endpoint = github.request.endpoint
90+
return endpoint({}).headers.accept
91+
result-encoding: string
92+
- id: previews-set-single
93+
name: Previews set to a single value
94+
uses: ./
95+
with:
96+
previews: foo
97+
script: |
98+
const endpoint = github.request.endpoint
99+
return endpoint({}).headers.accept
100+
result-encoding: string
101+
- id: previews-set-multiple
102+
name: Previews set to comma-separated list
103+
uses: ./
104+
with:
105+
previews: foo,bar,baz
106+
script: |
107+
const endpoint = github.request.endpoint
108+
return endpoint({}).headers.accept
109+
result-encoding: string
110+
- run: |
111+
echo "- Validating previews default"
112+
expected="application/vnd.github.v3+json"
113+
if [[ "${{steps.previews-default.outputs.result}}" != $expected ]]; then
114+
echo $'::error::\u274C' "Expected '$expected', got ${{steps.previews-default.outputs.result}}"
115+
exit 1
116+
fi
117+
echo "- Validating previews set to a single value"
118+
expected="application/vnd.github.foo-preview+json"
119+
if [[ "${{steps.previews-set-single.outputs.result}}" != $expected ]]; then
120+
echo $'::error::\u274C' "Expected '$expected', got ${{steps.previews-set-single.outputs.result}}"
121+
exit 1
122+
fi
123+
echo "- Validating previews set to multiple values"
124+
expected="application/vnd.github.foo-preview+json,application/vnd.github.bar-preview+json,application/vnd.github.baz-preview+json"
125+
if [[ "${{steps.previews-set-multiple.outputs.result}}" != $expected ]]; then
126+
echo $'::error::\u274C' "Expected '$expected', got ${{steps.previews-set-multiple.outputs.result}}"
127+
exit 1
128+
fi
129+
echo $'\u2705 Test passed' | tee -a $GITHUB_STEP_SUMMARY
130+
131+
test-user-agent:
132+
name: 'Integration test: user-agent option'
133+
runs-on: ubuntu-latest
134+
steps:
135+
- uses: actions/checkout@v3
136+
- uses: actions/cache@v3
137+
with:
138+
path: ~/.npm
139+
key: ${{runner.os}}-npm-${{hashFiles('**/package-lock.json')}}
140+
restore-keys: ${{runner.os}}-npm-
141+
- run: npm ci
142+
- id: user-agent-default
143+
name: Default user-agent not set
144+
uses: ./
145+
with:
146+
script: |
147+
const endpoint = github.request.endpoint
148+
return endpoint({}).headers['user-agent']
149+
result-encoding: string
150+
- id: user-agent-set
151+
name: User-agent set
152+
uses: ./
153+
with:
154+
user-agent: foobar
155+
script: |
156+
const endpoint = github.request.endpoint
157+
return endpoint({}).headers['user-agent']
158+
result-encoding: string
159+
- id: user-agent-empty
160+
name: User-agent set to an empty string
161+
uses: ./
162+
with:
163+
user-agent: ''
164+
script: |
165+
const endpoint = github.request.endpoint
166+
return endpoint({}).headers['user-agent']
167+
result-encoding: string
168+
- run: |
169+
echo "- Validating user-agent default"
170+
expected="actions/github-script octokit-core.js/"
171+
if [[ "${{steps.user-agent-default.outputs.result}}" != "$expected"* ]]; then
172+
echo $'::error::\u274C' "Expected user-agent to start with '$expected', got ${{steps.user-agent-default.outputs.result}}"
173+
exit 1
174+
fi
175+
echo "- Validating user-agent set to a value"
176+
expected="foobar octokit-core.js/"
177+
if [[ "${{steps.user-agent-set.outputs.result}}" != "$expected"* ]]; then
178+
echo $'::error::\u274C' "Expected user-agent to start with '$expected', got ${{steps.user-agent-set.outputs.result}}"
179+
exit 1
180+
fi
181+
echo "- Validating user-agent set to an empty string"
182+
expected="octokit-core.js/"
183+
if [[ "${{steps.user-agent-empty.outputs.result}}" != "$expected"* ]]; then
184+
echo $'::error::\u274C' "Expected user-agent to start with '$expected', got ${{steps.user-agent-empty.outputs.result}}"
185+
exit 1
186+
fi
187+
echo $'\u2705 Test passed' | tee -a $GITHUB_STEP_SUMMARY
188+
189+
test-debug:
190+
name: 'Integration test: debug option'
191+
runs-on: ubuntu-latest
192+
steps:
193+
- uses: actions/checkout@v3
194+
- uses: actions/cache@v3
195+
with:
196+
path: ~/.npm
197+
key: ${{runner.os}}-npm-${{hashFiles('**/package-lock.json')}}
198+
restore-keys: ${{runner.os}}-npm-
199+
- run: npm ci
200+
- id: debug-default
201+
name: Default debug not set
202+
uses: ./
203+
with:
204+
script: |
205+
const log = github.log
206+
return {
207+
debug: log.debug === console.debug,
208+
info: log.info === console.info
209+
}
210+
- id: debug-true
211+
name: Debug set to true
212+
uses: ./
213+
with:
214+
debug: true
215+
script: |
216+
const log = github.log
217+
return {
218+
debug: log.debug === console.debug,
219+
info: log.info === console.info
220+
}
221+
- id: debug-false
222+
name: Debug set to false
223+
uses: ./
224+
with:
225+
debug: false
226+
script: |
227+
const log = github.log
228+
return {
229+
debug: log.debug === console.debug,
230+
info: log.info === console.info
231+
}
232+
- run: |
233+
echo "- Validating debug default"
234+
expected='{debug:false,info:false}'
235+
if [[ "${{steps.debug-default.outputs.result}}" != "$expected" ]]; then
236+
echo $'::error::\u274C' "Expected '$expected', got ${{steps.debug-default.outputs.result}}"
237+
exit 1
238+
fi
239+
echo "- Validating debug set to true"
240+
expected='{debug:true,info:true}'
241+
if [[ "${{steps.debug-true.outputs.result}}" != "$expected" ]]; then
242+
echo $'::error::\u274C' "Expected '$expected', got ${{steps.debug-true.outputs.result}}"
243+
exit 1
244+
fi
245+
echo "- Validating debug set to false"
246+
expected='{debug:false,info:false}'
247+
if [[ "${{steps.debug-false.outputs.result}}" != "$expected" ]]; then
248+
echo $'::error::\u274C' "Expected '$expected', got ${{steps.debug-false.outputs.result}}"
249+
exit 1
250+
fi
251+
echo $'\u2705 Test passed' | tee -a $GITHUB_STEP_SUMMARY

‎dist/index.js

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15134,6 +15134,9 @@ var io = __nccwpck_require__(7436);
1513415134
var dist_node = __nccwpck_require__(8883);
1513515135
// EXTERNAL MODULE: ./node_modules/@octokit/plugin-retry/dist-node/index.js
1513615136
var plugin_retry_dist_node = __nccwpck_require__(6298);
15137+
// EXTERNAL MODULE: ./node_modules/node-fetch/lib/index.js
15138+
var lib = __nccwpck_require__(467);
15139+
var lib_default = /*#__PURE__*/__nccwpck_require__.n(lib);
1513715140
;// CONCATENATED MODULE: ./src/async-function.ts
1513815141
const AsyncFunction = Object.getPrototypeOf(async () => null).constructor;
1513915142
function callAsyncFunction(args, source) {
@@ -15161,7 +15164,7 @@ function getRetryOptions(retries, exemptStatusCodes, defaultOptions) {
1516115164
...defaultOptions.request,
1516215165
retries
1516315166
};
15164-
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]'})`);
15167+
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]'})`);
1516515168
return [retryOptions, requestOptions];
1516615169
}
1516715170
function parseNumberArray(listString) {
@@ -15197,9 +15200,6 @@ const wrapRequire = new Proxy(require, {
1519715200
}
1519815201
});
1519915202

15200-
// EXTERNAL MODULE: ./node_modules/node-fetch/lib/index.js
15201-
var lib = __nccwpck_require__(467);
15202-
var lib_default = /*#__PURE__*/__nccwpck_require__.n(lib);
1520315203
;// CONCATENATED MODULE: ./src/main.ts
1520415204

1520515205

@@ -15217,23 +15217,19 @@ process.on('unhandledRejection', handleError);
1521715217
main().catch(handleError);
1521815218
async function main() {
1521915219
const token = core.getInput('github-token', { required: true });
15220-
const debug = core.getInput('debug');
15220+
const debug = core.getBooleanInput('debug');
1522115221
const userAgent = core.getInput('user-agent');
1522215222
const previews = core.getInput('previews');
1522315223
const retries = parseInt(core.getInput('retries'));
1522415224
const exemptStatusCodes = parseNumberArray(core.getInput('retry-exempt-status-codes'));
1522515225
const [retryOpts, requestOpts] = getRetryOptions(retries, exemptStatusCodes, utils.defaults);
15226-
const opts = {};
15227-
if (debug === 'true')
15228-
opts.log = console;
15229-
if (userAgent != null)
15230-
opts.userAgent = userAgent;
15231-
if (previews != null)
15232-
opts.previews = previews.split(',');
15233-
if (retryOpts)
15234-
opts.retry = retryOpts;
15235-
if (requestOpts)
15236-
opts.request = requestOpts;
15226+
const opts = {
15227+
log: debug ? console : undefined,
15228+
userAgent: userAgent || undefined,
15229+
previews: previews ? previews.split(',') : undefined,
15230+
retry: retryOpts,
15231+
request: requestOpts
15232+
};
1523715233
const github = (0,lib_github.getOctokit)(token, opts, plugin_retry_dist_node/* retry */.XD, dist_node/* requestLog */.g);
1523815234
const script = core.getInput('script', { required: true });
1523915235
// Using property/value shorthand on `require` (e.g. `{require}`) causes compilation errors.

‎src/main.ts

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ import * as io from '@actions/io'
77
import {requestLog} from '@octokit/plugin-request-log'
88
import {retry} from '@octokit/plugin-retry'
99
import {RequestRequestOptions} from '@octokit/types'
10+
import fetch from 'node-fetch'
1011
import {callAsyncFunction} from './async-function'
11-
import {getRetryOptions, parseNumberArray, RetryOptions} from './retry-options'
12+
import {RetryOptions, getRetryOptions, parseNumberArray} from './retry-options'
1213
import {wrapRequire} from './wrap-require'
13-
import fetch from 'node-fetch'
1414

1515
process.on('unhandledRejection', handleError)
1616
main().catch(handleError)
@@ -25,7 +25,7 @@ type Options = {
2525

2626
async function main(): Promise<void> {
2727
const token D7AE = core.getInput('github-token', {required: true})
28-
const debug = core.getInput('debug')
28+
const debug = core.getBooleanInput('debug')
2929
const userAgent = core.getInput('user-agent')
3030
const previews = core.getInput('previews')
3131
const retries = parseInt(core.getInput('retries'))
@@ -38,12 +38,13 @@ async function main(): Promise<void> {
3838
defaultGitHubOptions
3939
)
4040

41-
const opts: Options = {}
42-
if (debug === 'true') opts.log = console
43-
if (userAgent != null) opts.userAgent = userAgent
44-
if (previews != null) opts.previews = previews.split(',')
45-
if (retryOpts) opts.retry = retryOpts
46-
if (requestOpts) opts.request = requestOpts
41+
const opts: Options = {
42+
log: debug ? console : undefined,
43+
userAgent: userAgent || undefined,
44+
previews: previews ? previews.split(',') : undefined,
45+
retry: retryOpts,
46+
request: requestOpts
47+
}
4748

4849
const github = getOctokit(token, opts, retry, requestLog)
4950
const script = core.getInput('script', {required: true})

‎src/retry-options.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ export function getRetryOptions(
3636
`GitHub client configured with: (retries: ${
3737
requestOptions.retries
3838
}, retry-exempt-status-code: ${
39-
retryOptions?.doNotRetry ?? 'octokit default: [400, 401, 403, 404, 422]'
39+
retryOptions.doNotRetry ?? 'octokit default: [400, 401, 403, 404, 422]'
4040
})`
4141
)
4242

‎tsconfig.eslint.json

Lines changed: 4 additions & 0 deletions
< 412F td data-grid-cell-id="diff-5053c0bda6e3ba2fcfa5bbe118ecf5c60fa9f099b78b3e6deb66e5679424f515-empty-2-2" data-line-anchor="diff-5053c0bda6e3ba2fcfa5bbe118ecf5c60fa9f099b78b3e6deb66e5679424f515R2" data-selected="false" role="gridcell" style="background-color:var(--diffBlob-additionLine-bgColor, var(--diffBlob-addition-bgColor-line));padding-right:24px" tabindex="-1" valign="top" class="focusable-grid-cell diff-text-cell right-side-diff-cell left-side">+
"extends": "./tsconfig.json",
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2
3+
"exclude": []
4+
}

0 commit comments

Comments
 (0)
0