From 2b9bd70e3ac95244db3a3cd57a587709ea6fcd92 Mon Sep 17 00:00:00 2001 From: Kristian Dimitrov Date: Mon, 19 Nov 2018 14:29:16 +0200 Subject: [PATCH 1/2] fix: plugin create doesn't always clean up folder if execution fails --- lib/commands/plugin/create-plugin.ts | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/lib/commands/plugin/create-plugin.ts b/lib/commands/plugin/create-plugin.ts index 423f0c815d..d83cb29da1 100644 --- a/lib/commands/plugin/create-plugin.ts +++ b/lib/commands/plugin/create-plugin.ts @@ -22,8 +22,17 @@ export class CreatePluginCommand implements ICommand { const selectedPath = path.resolve(pathToProject || "."); const projectDir = path.join(selectedPath, pluginRepoName); - await this.downloadPackage(selectedTemplate, projectDir); - await this.setupSeed(projectDir, pluginRepoName); + // Must be out of try catch block, because will throw error if folder alredy exists and we don't want to delete it. + this.ensurePackageDir(projectDir); + + try { + await this.downloadPackage(selectedTemplate, projectDir); + await this.setupSeed(projectDir, pluginRepoName); + } catch (err) { + // The call to this.ensurePackageDir() above will throw error if folder alredy exists, so it is safe to delete here. + this.$fs.deleteDirectory(projectDir); + throw err; + } this.$logger.printMarkdown("Solution for `%s` was successfully created.", pluginRepoName); } @@ -66,13 +75,15 @@ export class CreatePluginCommand implements ICommand { } } - private async downloadPackage(selectedTemplate: string, projectDir: string): Promise { + private ensurePackageDir(projectDir: string): void { this.$fs.createDirectory(projectDir); if (this.$fs.exists(projectDir) && !this.$fs.isEmptyDir(projectDir)) { this.$errors.fail("Path already exists and is not empty %s", projectDir); } + } + private async downloadPackage(selectedTemplate: string, projectDir: string): Promise { if (selectedTemplate) { this.$logger.printMarkdown("Make sure your custom template is compatible with the Plugin Seed at https://github.com/NativeScript/nativescript-plugin-seed/"); } else { @@ -84,9 +95,6 @@ export class CreatePluginCommand implements ICommand { try { spinner.start(); await this.$pacoteService.extractPackage(packageToInstall, projectDir); - } catch (err) { - this.$fs.deleteDirectory(projectDir); - throw err; } finally { spinner.stop(); } From 57b1631cf4e989f2056831a380cff51de41e9ed9 Mon Sep 17 00:00:00 2001 From: Kristian Dimitrov Date: Mon, 19 Nov 2018 18:13:45 +0200 Subject: [PATCH 2/2] test: add tests for plugin dir removal --- lib/commands/plugin/create-plugin.ts | 3 +- test/plugin-create.ts | 65 +++++++++++++++++++++++++++- 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/lib/commands/plugin/create-plugin.ts b/lib/commands/plugin/create-plugin.ts index d83cb29da1..3eb4baa121 100644 --- a/lib/commands/plugin/create-plugin.ts +++ b/lib/commands/plugin/create-plugin.ts @@ -5,6 +5,7 @@ export class CreatePluginCommand implements ICommand { public allowedParameters: ICommandParameter[] = []; public userMessage = "What is your GitHub username?\n(will be used to update the Github URLs in the plugin's package.json)"; public nameMessage = "What will be the name of your plugin?\n(use lowercase characters and dashes only)"; + public pathAlreadyExistsMessageTemplate = "Path already exists and is not empty %s"; constructor(private $options: IOptions, private $errors: IErrors, private $terminalSpinnerService: ITerminalSpinnerService, @@ -79,7 +80,7 @@ export class CreatePluginCommand implements ICommand { this.$fs.createDirectory(projectDir); if (this.$fs.exists(projectDir) && !this.$fs.isEmptyDir(projectDir)) { - this.$errors.fail("Path already exists and is not empty %s", projectDir); + this.$errors.fail(this.pathAlreadyExistsMessageTemplate, projectDir); } } diff --git a/test/plugin-create.ts b/test/plugin-create.ts index c6ab5b6f6b..a59e42f5a5 100644 --- a/test/plugin-create.ts +++ b/test/plugin-create.ts @@ -3,6 +3,11 @@ import * as stubs from "./stubs"; import { CreatePluginCommand } from "../lib/commands/plugin/create-plugin"; import { assert } from "chai"; import helpers = require("../lib/common/helpers"); +import * as sinon from "sinon"; +import temp = require("temp"); +import * as path from "path"; +import * as util from "util"; +temp.track(); interface IPacoteOutput { packageName: string; @@ -10,7 +15,8 @@ interface IPacoteOutput { } const originalIsInteractive = helpers.isInteractive; -const dummyArgs = ["dummyProjectName"]; +const dummyProjectName = "dummyProjectName"; +const dummyArgs = [dummyProjectName]; const dummyUser = "devUsername"; const dummyName = "devPlugin"; const dummyPacote: IPacoteOutput = { packageName: "", destinationDirectory: "" }; @@ -142,5 +148,62 @@ describe("Plugin create command tests", () => { options.pluginName = dummyName; await createPluginCommand.execute(dummyArgs); }); + + describe("when fails", () => { + let sandbox: sinon.SinonSandbox; + let fsSpy: sinon.SinonSpy; + let projectPath: string; + + beforeEach(() => { + sandbox = sinon.sandbox.create(); + const workingPath = temp.mkdirSync("test_plugin"); + options.path = workingPath; + projectPath = path.join(workingPath, dummyProjectName); + const fsService = testInjector.resolve("fs"); + fsSpy = sandbox.spy(fsService, "deleteDirectory"); + }); + + afterEach(() => { + sandbox.restore(); + }); + + it("downloadPackage, should remove projectDir", async () => { + const errorMessage = "Test fail"; + const pacoteService = testInjector.resolve("pacoteService"); + sandbox.stub(pacoteService, "extractPackage").callsFake(() => { + return Promise.reject(new Error(errorMessage)); + }); + + const executePromise = createPluginCommand.execute(dummyArgs); + + await assert.isRejected(executePromise, errorMessage); + assert(fsSpy.calledWith(projectPath)); + }); + + it("setupSeed, should remove projectDir", async () => { + const errorMessage = "Test fail"; + const npmService = testInjector.resolve("npm"); + sandbox.stub(npmService, "install").callsFake(() => { + return Promise.reject(new Error(errorMessage)); + }); + + const executePromise = createPluginCommand.execute(dummyArgs); + + await assert.isRejected(executePromise, errorMessage); + assert(fsSpy.calledWith(projectPath)); + }); + + it("ensurePachageDir should not remove projectDir", async () => { + const fsService = testInjector.resolve("fs"); + sandbox.stub(fsService, "isEmptyDir").callsFake(() => { + return false; + }); + + const executePromise = createPluginCommand.execute(dummyArgs); + + await assert.isRejected(executePromise, util.format(createPluginCommand.pathAlreadyExistsMessageTemplate, projectPath)); + assert(fsSpy.notCalled); + }); + }); }); });