From 7b4b086c7e98e9a1a29643886dc53d962927968d Mon Sep 17 00:00:00 2001 From: Glenn Hinks Date: Wed, 29 Dec 2021 11:51:02 -0500 Subject: [PATCH 1/5] fix: close-pr count included unsuccessful runs Changed the result to look at whether the array actually had a length as on testing I found that it always said "1 PRs closed" when closing a failing PR. --- lib/closePR.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/closePR.js b/lib/closePR.js index 6239d4e..e8666ec 100644 --- a/lib/closePR.js +++ b/lib/closePR.js @@ -16,7 +16,7 @@ module.exports = async ({ dependents }) => { const branch = await context.getTestingBranchName(parentBranchName) const resp = await github.getChecks(dependentPkgInfo.owner, dependentPkgInfo.name, branch) const closedPR = await closeDependencyPR(dependentPkgInfo.owner, dependentPkgInfo.name, branch, resp.data.check_runs) - if (closedPR) { + if (closedPR && closedPR.length > 0) { closedPrs.push(closedPR) } } From 1d8146d1d603b59f2080d055934e8a151d5a0e89 Mon Sep 17 00:00:00 2001 From: Glenn Hinks Date: Fri, 31 Dec 2021 14:37:49 -0500 Subject: [PATCH 2/5] fix: array sz zero handling assume an array will always be returned --- lib/closePR.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/closePR.js b/lib/closePR.js index e8666ec..ee9678c 100644 --- a/lib/closePR.js +++ b/lib/closePR.js @@ -16,7 +16,7 @@ module.exports = async ({ dependents }) => { const branch = await context.getTestingBranchName(parentBranchName) const resp = await github.getChecks(dependentPkgInfo.owner, dependentPkgInfo.name, branch) const closedPR = await closeDependencyPR(dependentPkgInfo.owner, dependentPkgInfo.name, branch, resp.data.check_runs) - if (closedPR && closedPR.length > 0) { + if (closedPR.length > 0) { closedPrs.push(closedPR) } } From e7bdc2ac052a60b971ad05458aa66b50a368c654 Mon Sep 17 00:00:00 2001 From: Glenn Hinks Date: Wed, 5 Jan 2022 11:44:13 -0500 Subject: [PATCH 3/5] fix: array sz zero handling the array length needs to be checked as an array that is empty will be returned by the reduce and promise all combination --- lib/closePR.js | 2 +- test/cli/closePR.js | 10 +++++ .../http/close-pr-command-no-positive.js | 45 +++++++++++++++++++ 3 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 test/fixtures/http/close-pr-command-no-positive.js diff --git a/lib/closePR.js b/lib/closePR.js index ee9678c..f8d8045 100644 --- a/lib/closePR.js +++ b/lib/closePR.js @@ -16,7 +16,7 @@ module.exports = async ({ dependents }) => { const branch = await context.getTestingBranchName(parentBranchName) const resp = await github.getChecks(dependentPkgInfo.owner, dependentPkgInfo.name, branch) const closedPR = await closeDependencyPR(dependentPkgInfo.owner, dependentPkgInfo.name, branch, resp.data.check_runs) - if (closedPR.length > 0) { + if (closedPR && closedPR.length) { closedPrs.push(closedPR) } } diff --git a/test/cli/closePR.js b/test/cli/closePR.js index 02ce01b..c187380 100644 --- a/test/cli/closePR.js +++ b/test/cli/closePR.js @@ -40,4 +40,14 @@ tap.test('closePRs command', async (t) => { }).toString() t.match(result, '1 PRs closed\n') }) + t.test('close-pr with no result as PR was failing', async (t) => { + const result = childProcess.execSync(`${wibyCommand} close-pr`, { + env: { + ...process.env, + NODE_OPTIONS: `-r ${fixturesPath}/http/close-pr-command-no-positive.js` + } + }).toString() + console.log(`pattern was => ${result}`) + t.match(result, '0 PRs closed\n') + }) }) diff --git a/test/fixtures/http/close-pr-command-no-positive.js b/test/fixtures/http/close-pr-command-no-positive.js new file mode 100644 index 0000000..1c7fa00 --- /dev/null +++ b/test/fixtures/http/close-pr-command-no-positive.js @@ -0,0 +1,45 @@ +'use strict' + +/* + Mocks of HTTP calls for close-pr command + */ +const nock = require('nock') + +nock.disableNetConnect() + +function nockRepo (nockInstance, owner, repo, branch) { + return nockInstance + // /repos/{owner}/{repo}/commits/{ref}/check-runs + .get(`/repos/${owner}/${repo}/commits/${branch}/check-runs`) + .reply(200, { + check_runs: [ + { + status: 'completed', + conclusion: 'failure', + pull_requests: [{ + number: 1, + head: { + ref: branch + } + }, { + status: 'completed', + conclusion: 'failure', + number: 2, + head: { + ref: 'any-other-branch' + } + }] + } + ] + }) +} + +function buildNock () { + let nockInstance = nock('https://api.github.com') + + nockInstance = nockRepo(nockInstance, 'wiby-test', 'fakeRepo', 'wiby-running-unit-tests') + nockInstance = nockRepo(nockInstance, 'wiby-test', 'pass', 'wiby-running-unit-tests') + return nockInstance +} + +buildNock() From 798292d05e26adba6ad36efb79764920ab438c9c Mon Sep 17 00:00:00 2001 From: Glenn Hinks Date: Sat, 15 Jan 2022 11:24:25 -0500 Subject: [PATCH 4/5] fix: grammar Change code per suggestions. Using ? --- lib/closePR.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/closePR.js b/lib/closePR.js index f8d8045..765de10 100644 --- a/lib/closePR.js +++ b/lib/closePR.js @@ -16,7 +16,7 @@ module.exports = async ({ dependents }) => { const branch = await context.getTestingBranchName(parentBranchName) const resp = await github.getChecks(dependentPkgInfo.owner, dependentPkgInfo.name, branch) const closedPR = await closeDependencyPR(dependentPkgInfo.owner, dependentPkgInfo.name, branch, resp.data.check_runs) - if (closedPR && closedPR.length) { + if (closedPR?.length > 0) { closedPrs.push(closedPR) } } From ea96251053f521138e28aa719c24a42a62ade8ab Mon Sep 17 00:00:00 2001 From: Glenn Hinks Date: Sat, 15 Jan 2022 11:29:53 -0500 Subject: [PATCH 5/5] fix: grammar Optional chaining was available from node 14 onwards, we still support node 12 and so although I'm happy with the change we would have to stop supporting node 12. --- lib/closePR.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/closePR.js b/lib/closePR.js index 765de10..f8d8045 100644 --- a/lib/closePR.js +++ b/lib/closePR.js @@ -16,7 +16,7 @@ module.exports = async ({ dependents }) => { const branch = await context.getTestingBranchName(parentBranchName) const resp = await github.getChecks(dependentPkgInfo.owner, dependentPkgInfo.name, branch) const closedPR = await closeDependencyPR(dependentPkgInfo.owner, dependentPkgInfo.name, branch, resp.data.check_runs) - if (closedPR?.length > 0) { + if (closedPR && closedPR.length) { closedPrs.push(closedPR) } }