Merge pull request #101 from pkgjs/reuse-branch

lib/github.js

@@ -59,7 +59,7 @@ module.exports.getShas = async function getShas (owner, repo) {
   })
   const headSha = resp.data[0].sha
   const treeSha = resp.data[0].commit.tree.sha
-  return [headSha, treeSha]
+  return { headSha, treeSha }
 }
 
 module.exports.createBlob = async function createBlob (owner, repo, file) {
@@ -117,11 +117,13 @@ module.exports.deleteBranch = async function deleteBranch (owner, repo, branch)
 
 module.exports.getBranch = async function getBranch (owner, repo, branch) {
   try {
-    return await octokit.repos.getBranch({
+    const { data } = await octokit.repos.getBranch({
       owner,
       repo,
       branch
     })
+
+    return data
   } catch (err) {
     if (err.status === 404) {
       return undefined
@@ -130,6 +132,12 @@ module.exports.getBranch = async function getBranch (owner, repo, branch) {
   }
 }
 
+module.exports.getPullsForBranch = async function getBranch (owner, repo, branch) {
+  const { data } = await octokit.pulls.list({ owner, repo, head: `${owner}:${branch}` })
+
+  return data
+}
+
 module.exports.getChecks = async function getChecks (owner, repo, branch) {
   return octokit.checks.listForRef({
     owner,
@@ -166,3 +174,12 @@ module.exports.closePR = async function closePR (owner, repo, pullNumber) {
     state: 'closed'
   })
 }
+
+module.exports.updateRef = async function updateRef (owner, repo, ref, sha) {
+  return octokit.rest.git.updateRef({
+    owner,
+    repo,
+    ref,
+    sha
+  })
+}

lib/test.js

@@ -52,22 +52,43 @@ async function pushPatch (dependentPkgJson, dependentOwner, dependentRepo, paren
   const file = JSON.stringify(dependentPkgJson, null, 2) + '\n' // assumes package.json is using two spaces
   const encodedFile = Buffer.from(file).toString('base64')
   const message = `wiby: update ${parentName}`
-  const branch = await context.getTestingBranchName(parentBranchName)
+  const testingBranchName = await context.getTestingBranchName(parentBranchName)
 
-  const [headSha, treeSha] = await github.getShas(dependentOwner, dependentRepo)
+  const testingBranch = await github.getBranch(dependentOwner, dependentRepo, testingBranchName)
   const blobSha = await github.createBlob(dependentOwner, dependentRepo, encodedFile)
-  const newTreeSha = await github.createTree(dependentOwner, dependentRepo, treeSha, blobSha)
-  const commitSha = await github.createCommit(dependentOwner, dependentRepo, message, newTreeSha, headSha)
-  await github.createBranch(dependentOwner, dependentRepo, commitSha, branch)
-  debug(`Changes pushed to https://github.com/${dependentOwner}/${dependentRepo}/blob/${branch}/package.json`)
+
+  if (!testingBranch) {
+    // testing branch does not yet exist - create it
+    const { headSha, treeSha } = await github.getShas(dependentOwner, dependentRepo)
+    const newTreeSha = await github.createTree(dependentOwner, dependentRepo, treeSha, blobSha)
+    const commitSha = await github.createCommit(dependentOwner, dependentRepo, message, newTreeSha, headSha)
+    await github.createBranch(dependentOwner, dependentRepo, commitSha, testingBranchName)
+
+    debug(`Changes pushed to https://github.com/${dependentOwner}/${dependentRepo}/blob/${testingBranchName}/package.json`)
+  } else {
+    // testing branch already exists - ensure we have the package.json patched or at least push an empty commit
+    const { sha: headSha, commit: { tree: { sha: treeSha } } } = testingBranch.commit
+    const newTreeSha = await github.createTree(dependentOwner, dependentRepo, treeSha, blobSha)
+    const commitSha = await github.createCommit(dependentOwner, dependentRepo, message, newTreeSha, headSha)
+    await github.updateRef(dependentOwner, dependentRepo, `heads/${testingBranchName}`, commitSha)
+
+    debug(`Pushed a new commit to https://github.com/${dependentOwner}/${dependentRepo}#${testingBranchName}`)
+  }
 }
 
-const createPR = module.exports.createPR = async function createPR (dependentOwner, dependentRepo, parentBranchName, parentDependencyLink) {
+async function createPR (dependentOwner, dependentRepo, parentBranchName, parentDependencyLink) {
   const title = `Wiby changes to ${parentDependencyLink}`
   const body = `Wiby has raised this PR to kick off automated jobs.
   You are dependent upon ${parentDependencyLink}
   `
-  const head = context.getTestingBranchName(parentBranchName)
+  const testingBranchName = context.getTestingBranchName(parentBranchName)
+
+  const pullsForTestingBranch = await github.getPullsForBranch(dependentOwner, dependentRepo, testingBranchName)
+  if (pullsForTestingBranch.length) {
+    debug(`Existing PRs updated: ${pullsForTestingBranch.map((data) => data.html_url).join(', ')}`)
+    return
+  }
+
   const draft = true
   /*
         from the conversation on wiby PR 93 https://github.com/pkgjs/wiby/pull/93#discussion_r615362448
@@ -78,11 +99,12 @@ const createPR = module.exports.createPR = async function createPR (dependentOwn
   const preExistingOnDependent = await github.getBranch(dependentOwner, dependentRepo, parentBranchName)
   let result
   if (preExistingOnDependent) {
-    result = await github.createPR(dependentOwner, dependentRepo, title, head, parentBranchName, draft, body)
+    result = await github.createPR(dependentOwner, dependentRepo, title, testingBranchName, parentBranchName, draft, body)
+    debug(`PR raised upon ${result.data.html_url} (base branch: ${parentBranchName})`)
   } else {
     const defaultBranch = await github.getDefaultBranch(dependentOwner, dependentRepo)
-    result = await github.createPR(dependentOwner, dependentRepo, title, head, defaultBranch, draft, body)
+    result = await github.createPR(dependentOwner, dependentRepo, title, testingBranchName, defaultBranch, draft, body)
+    debug(`PR raised upon ${result.data.html_url} (base branch: ${defaultBranch})`)
   }
-  debug(`PR raised upon ${result.data.html_url}`)
   return result
 }

test/cli/test.js

@@ -47,6 +47,23 @@ tap.test('test command', async (tap) => {
     tap.match(result, 'Changes pushed to https://github.com/wiby-test/pass/blob/wiby-running-unit-tests/package.json')
     tap.match(result, 'Changes pushed to https://github.com/wiby-test/fail/blob/wiby-running-unit-tests/package.json')
     tap.match(result, 'Changes pushed to https://github.com/wiby-test/partial/blob/wiby-running-unit-tests/package.json')
+    tap.match(result, 'PR raised upon https://github.com/pkgjs/wiby/pull/1 (base branch: running-unit-tests)')
+  })
+
+  tap.test('test command should update existing wiby test branches', async (tap) => {
+    gitFixture.init('existing-branch')
+
+    const result = childProcess.execSync(`${wibyCommand} test`, {
+      env: {
+        ...process.env,
+        NODE_OPTIONS: `-r ${fixturesPath}/http/test-command-positive.js`
+      }
+    }).toString()
+
+    tap.match(result, 'Pushed a new commit to https://github.com/wiby-test/pass#wiby-existing-branch')
+    tap.match(result, 'Pushed a new commit to https://github.com/wiby-test/fail#wiby-existing-branch')
+    tap.match(result, 'Pushed a new commit to https://github.com/wiby-test/partial#wiby-existing-branch')
+    tap.match(result, 'Existing PRs updated: https://github.com/pkgjs/wiby/pull/1')
   })
 
   tap.test('test command should not add `wiby-` prefix when branch already has it', async (tap) => {
@@ -62,5 +79,6 @@ tap.test('test command', async (tap) => {
     tap.match(result, 'Changes pushed to https://github.com/wiby-test/pass/blob/wiby-running-unit-tests/package.json')
     tap.match(result, 'Changes pushed to https://github.com/wiby-test/fail/blob/wiby-running-unit-tests/package.json')
     tap.match(result, 'Changes pushed to https://github.com/wiby-test/partial/blob/wiby-running-unit-tests/package.json')
+    tap.match(result, 'PR raised upon https://github.com/pkgjs/wiby/pull/1 (base branch: main)')
   })
 })

test/closePR.js

@@ -41,9 +41,7 @@ tap.test('close PR', (t) => {
       nock('https://api.github.com')
         // get package json
         .patch('/repos/pkgjs/wiby/pulls/1')
-        .reply(200, {
-          data: {}
-        })
+        .reply(200, {})
       const result = await closePR.closeDependencyPR(owner, repo, branch, [{
         status: 'completed',
         conclusion: 'success',
@@ -68,9 +66,7 @@ tap.test('close PR', (t) => {
       // nock setup
       nock('https://api.github.com')
         .get(/repos.*check-runs/)
-        .reply(200, {
-          data: {}
-        })
+        .reply(200, {})
       const result = await closePR({
         dependents: [
           {

test/fixtures/http/clean-command-dry.js

@@ -10,7 +10,7 @@ nock.disableNetConnect()
 function nockRepo (nockInstance, repo) {
   return nockInstance
     .get(`/repos/wiby-test/${repo}/branches/wiby-running-unit-tests`)
-    .reply(200)
+    .reply(200, {})
 }
 
 function buildNock () {

test/fixtures/http/clean-command.js

@@ -10,7 +10,7 @@ nock.disableNetConnect()
 function nockRepo (nockInstance, repo) {
   return nockInstance
     .get(`/repos/wiby-test/${repo}/branches/wiby-running-unit-tests`)
-    .reply(200)
+    .reply(200, {})
     .delete(`/repos/wiby-test/${repo}/git/refs/heads%2Fwiby-running-unit-tests`)
     .reply(200)
 }

test/fixtures/http/result-command-missing-branch.js

@@ -31,7 +31,7 @@ nock('https://api.github.com')
   .get('/repos/wiby-test/pass/branches/wiby-running-unit-tests')
   .reply(200, {})
   .get('/repos/wiby-test/fail/branches/wiby-running-unit-tests')
-  .reply(404, {})
+  .reply(404)
   // get check results
   .get('/repos/wiby-test/fakeRepo/commits/wiby-running-unit-tests/check-runs')
   .reply(200, {

test/fixtures/http/test-command-positive.js

@@ -49,7 +49,7 @@ function nockPkgjsWiby (nockInstance) {
       }
     ])
     .get('/repos/wiby-test/pass/branches/wiby-running-unit-tests')
-    .reply(404, {})
+    .reply(404)
 }
 
 function nockRepo (nockInstance, repo) {
@@ -89,10 +89,44 @@ function nockRepo (nockInstance, repo) {
     .reply(201, {
       html_url: 'https://github.com/pkgjs/wiby/pull/1'
     })
-    .get('/repos/wiby-test/pass/branches/running-unit-tests')
+    .get(`/repos/wiby-test/${repo}/branches/running-unit-tests`)
     .reply(200, {
-      name: 'running-unit-tests'
+      name: 'running-unit-tests',
+      commit: {
+        sha: 'head_sha',
+        commit: {
+          tree: {
+            sha: 'tree_sha'
+          }
+        }
+      }
+    })
+    .get(`/repos/wiby-test/${repo}/branches/wiby-running-unit-tests`)
+    .reply(404)
+    .get(`/repos/wiby-test/${repo}/branches/existing-branch`)
+    .reply(404)
+    .get(`/repos/wiby-test/${repo}/branches/wiby-existing-branch`)
+    .reply(200, {
+      name: 'wiby-existing-branch',
+      commit: {
+        sha: 'head_sha',
+        commit: {
+          tree: {
+            sha: 'tree_sha'
+          }
+        }
+      }
     })
+    .patch(`/repos/wiby-test/${repo}/git/refs/heads%2Fwiby-existing-branch`, { sha: 'fake_sha' })
+    .reply(204)
+    .get(`/repos/wiby-test/${repo}/pulls?head=wiby-test%3Awiby-running-unit-tests`)
+    .reply(200, [])
+    .get(`/repos/wiby-test/${repo}/pulls?head=wiby-test%3Awiby-existing-branch`)
+    .reply(200, [
+      {
+        html_url: 'https://github.com/pkgjs/wiby/pull/1'
+      }
+    ])
 }
 
 function buildNock () {

test/internals/github-integration.js

@@ -11,7 +11,7 @@ tap.test('package.json can be fetched with a valid url', async tap => {
 }, { skip: !process.env.GITHUB_TOKEN })
 
 tap.test('Shas returned from getShas', async tap => {
-  const [headSha, treeSha] = await github.getShas('pkgjs', 'wiby')
+  const { headSha, treeSha } = await github.getShas('pkgjs', 'wiby')
   tap.notEqual(headSha, null)
   tap.notEqual(treeSha, null)
 }, { skip: !process.env.GITHUB_TOKEN })

test/test.js

@@ -97,50 +97,4 @@ tap.test('wiby.test()', async (tap) => {
     )
     tap.end()
   })
-  tap.test('Create PR', (tap) => {
-    tap.plan(2)
-    const htmlURL = `https://github.com/${CONFIG.PKG_ORG}/${CONFIG.DEP_REPO}/pull/1`
-    const dependentOwner = 'pkgjs'
-    const dependentRepo = 'wiby'
-    const parentBranchName = 'main'
-    tap.beforeEach(() => {
-      nock('https://api.github.com')
-        .post('/repos/pkgjs/wiby/pulls')
-        .reply(201, {
-          html_url: htmlURL
-        })
-        .post(/graphql/)
-        .reply(200, {
-          data: {
-            repository: {
-              defaultBranchRef: {
-                name: 'main'
-              }
-            }
-          }
-        })
-    })
-    tap.test('test create PR when dependency feature branch does not exist in dependent repo', (t) => {
-      nock('https://api.github.com')
-        .get(/repos/)
-        .reply(404, {})
-      wiby.test.createPR(dependentOwner, dependentRepo, parentBranchName)
-        .then((result) => {
-          t.equal(result.data.html_url, htmlURL)
-          t.end()
-        })
-    })
-    tap.test('test create PR when dependency feature branch exists in dependent repo', (t) => {
-      nock('https://api.github.com')
-        .get(/repos/)
-        .reply(200, {
-          name: parentBranchName
-        })
-      wiby.test.createPR(dependentOwner, dependentRepo, parentBranchName)
-        .then((result) => {
-          t.equal(result.data.html_url, htmlURL)
-          t.end()
-        })
-    })
-  })
 })