fix breaking change can't find file bug (#38)

* fix breaking change can't find file bug

* add unit-test

* add comments for test

* update changelog

* add root cause

* add cleanUp function

* ignore rm error

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Changelog
 
+## 0.6.7
+
+- Fixed breaking change can't find file bug. doOnTargetBranch execute function until checkout branch finished.
+- The root cause is the switch branch function don't use await to pause execution process. The next code block execute directly and doesn't wait switch branch function finished, as a result it actually doesn't run on target branch.
+
 ## 0.6.6
 
 - Fixed octokit.issues undefined issue.

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@azure/rest-api-specs-scripts",
-  "version": "0.6.6",
+  "version": "0.6.7",
   "description": "Scripts for the Azure RestAPI specification repository 'azure-rest-api-specs'.",
   "types": "dist/index.d.ts",
   "main": "dist/index.js",

src/tests/helper.ts

@@ -0,0 +1,131 @@
+import * as path from "path";
+import * as pfs from "@ts-common/fs";
+import { git, cli, devOps, avocado } from "@azure/avocado";
+
+export const create = async (rootName: string, repoName: string) => {
+  const tmpRoot = path.resolve(path.join("..", rootName));
+  if (!(await pfs.exists(tmpRoot))) {
+    await pfs.mkdir(tmpRoot);
+  }
+  const tmp = path.join(tmpRoot, repoName);
+
+  cleanUp(rootName, repoName)
+  await pfs.mkdir(tmp);
+  return tmp;
+};
+
+export const cleanUp = async (rootName: string, repoName: string) => {
+  const tmpRoot = path.resolve(path.join("..", rootName));
+  const tmp = path.join(tmpRoot, repoName);
+  try {
+    if(await pfs.exists(tmpRoot)){
+      await pfs.recursiveRmdir(tmp);
+    }
+  } catch (error) {
+    //Force rm and ignore rm non-existing file error.
+  }
+}
+
+export const createDevOpsEnv = async (rootName: string, repoName: string): Promise<cli.Config> => {
+  const tmp = await create(rootName, repoName);
+
+  // Create '"${tmp}/remote"' folder.
+  const remote = path.join(tmp, "remote");
+  await pfs.mkdir(remote);
+
+  const gitRemote = git.repository(remote);
+
+  // create a Git repository
+  await gitRemote({ init: [] });
+  await gitRemote({ config: ["user.email", "test@example.com"] });
+  await gitRemote({ config: ["user.name", "test"] });
+
+  // commit invalid 'specification/readme.md' to 'master'.
+  const specification = path.join(remote, "specification");
+  await pfs.mkdir(specification);
+  await pfs.writeFile(path.join(specification, "readme.md"), "");
+  await pfs.writeFile(
+    path.join(specification, "file1.json"),
+    `
+      {
+        "a": "foo",
+        "b": [
+          "bar1",
+          "bar2",
+          "bar3"
+        ]
+      }
+      `
+  );
+
+  await pfs.writeFile(
+    path.join(specification, "file2.json"),
+    `
+      {
+        "a": "foo"
+      }
+      `
+  );
+
+  await pfs.writeFile(
+    path.join(specification, "file3.json"),
+    `
+      {
+        "a": "foo"
+      }
+      `
+  );
+
+  await pfs.writeFile(path.join(remote, "license"), "");
+  await gitRemote({ add: ["."] });
+  await gitRemote({ commit: ["-m", '"initial commit"', "--no-gpg-sign"] });
+
+  // commit removing 'specification/readme.md' to 'source'.
+  await gitRemote({ checkout: ["-b", "source"] });
+  await pfs.unlink(path.join(specification, "readme.md"));
+  await pfs.writeFile(
+    path.join(specification, "file1.json"),
+    `
+      {
+        "a": "foo",
+        "b": ["bar1","bar2","bar3"]
+      }
+      `
+  );
+
+  await pfs.writeFile(
+    path.join(specification, "file2.json"),
+    `
+      {
+        "a": "foo",
+        "b": "bar"
+      }
+      `
+  );
+
+  // file with invalid JSON
+  await pfs.writeFile(path.join(specification, "file3.json"), `random string`);
+
+  // json file that did not exist
+  await pfs.writeFile(path.join(specification, "file4.json"), `{"foo":"bar"}`);
+
+  await pfs.writeFile(path.join(remote, "textfile.txt"), "");
+  await pfs.writeFile(path.join(remote, "license"), "MIT");
+  await gitRemote({ add: ["."] });
+  await gitRemote({
+    commit: ["-m", '"second commit"', "--no-gpg-sign"]
+  });
+
+  // create local Git repository
+  const local = path.join(tmp, "local");
+  await pfs.mkdir(local);
+  const gitLocal = git.repository(local);
+  await gitLocal({ clone: ["../remote", "."] });
+
+  return {
+    cwd: local,
+    env: {
+      SYSTEM_PULLREQUEST_TARGETBRANCH: "master"
+    }
+  };
+};

src/tests/utilsTest.ts

@@ -1,9 +1,14 @@
+import { cleanUp } from './helper';
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License. See License in the project root for license information.
 
 import { suite, test, slow, timeout, skip, only } from "mocha-typescript";
+import { devOps } from '@azure/avocado';
 import * as assert from "assert";
 import {utils as utils} from "../index"
+import {createDevOpsEnv} from "./helper"
+import * as fs from 'fs-extra'
+
 
 @suite class UtilsTest {
     @test async "TestGetOpenapiTypeDataplane" () {
@@ -30,4 +35,41 @@ import {utils as utils} from "../index"
         let openapiType = await utils.getOpenapiType("/home/work/1/spec/specification/test/data-plane/test/readme.md")
         assert.equal(openapiType,"data-plane")
     }
+
+    @test async "TestDoOnTargetBranch" () {
+        const rootName = 'test-root'
+        const repoName = 'mock-repo'
+        const cfg = await createDevOpsEnv(rootName, repoName);
+        /** Create a mock pr.
+         * The pr contains two branches.
+         * Master:
+         * ├── license
+         * └── specification
+         *     ├── file1.json
+         *     ├── file2.json
+         *     ├── file3.json
+         *     └── readme.md
+         *
+         * Source:
+         * ├── license
+         * ├── specification
+         * │   ├── file1.json
+         * │   ├── file2.json
+         * │   ├── file3.json
+         * │   └── file4.json (new file)
+         * └── textfile.txt
+         * 
+         * */ 
+        const pr = await devOps.createPullRequestProperties(cfg)
+
+        const files = ['specification/file1.json', 'specification/file2.json', 'specification/file3.json', 'specification/file4.json']
+        if(pr!==undefined){
+            const newSwaggers = await utils.doOnTargetBranch(pr, async ()=>{
+                return files.filter(s=>!fs.existsSync(s))
+            })
+            assert.deepEqual(newSwaggers, ['specification/file4.json'])
+        }
+
+        await cleanUp(rootName, repoName)
+    }
 }

src/utils.ts

@@ -89,7 +89,7 @@ export const getTargetBranch = function() {
 export const doOnTargetBranch = async <T>(pr: devOps.PullRequestProperties, func: () => Promise<T>) => {
   const currentDir = process.cwd();
 
-  pr.checkout(pr.targetBranch)
+  await pr.checkout(pr.targetBranch)
 
   console.log(`Changing directory and executing the function...`);
   // pr.workingDir is a directory of a cloned Pull Request Git repository. We can't use