Code review comments for Maven and Gradle file system interaction improvements.

Author: HSAR

Tasks/Gradle/CodeAnalysis/Common/FileSystemInteractions.ts

@@ -5,6 +5,15 @@ import fs = require('fs');
 import tl = require('vsts-task-lib/task');
 
 export class FileSystemInteractions {
+
+    /**
+     * Copies a file from the source path to the destination path.
+     * If the destination path already exists, it is replaced. Otherwise a new file is created.
+     *
+     * Adapted from: https://github.com/Microsoft/vsts-task-lib/blob/master/node/task.ts
+     * @param sourcePath      Path to copy from
+     * @param destinationPath Path to copy to
+     */
     public static copyFile(sourcePath:string, destinationPath:string):void {
         shell.cp('-f', sourcePath, destinationPath);
 
@@ -13,7 +22,9 @@ export class FileSystemInteractions {
 
     /**
      * Create a directory at the specified path, including any folders in between.
-     * @param directoryPath Path to create.
+     *
+     * Copied from: https://github.com/Microsoft/vsts-task-lib/blob/master/node/task.ts
+     * @param directoryPath Path to create
      */
     public static createDirectory(directoryPath:string):void {
         // build a stack of directories to create
@@ -72,7 +83,14 @@ export class FileSystemInteractions {
         }
     }
 
-    public static checkShell(cmd: string, continueOnError?: boolean) {
+    /**
+     * Checks that no errors were produced by the previous command.
+     *
+     * Copied from: https://github.com/Microsoft/vsts-task-lib/blob/master/node/task.ts
+     * @param cmd             Command issued
+     * @param continueOnError Do not throw an exception if an error was produced
+     */
+    private static checkShell(cmd: string, continueOnError?: boolean) {
         var se = shell.error();
 
         if (se) {

Tasks/Maven/CodeAnalysis/Common/FileSystemInteractions.ts

@@ -5,6 +5,15 @@ import fs = require('fs');
 import tl = require('vsts-task-lib/task');
 
 export class FileSystemInteractions {
+
+    /**
+     * Copies a file from the source path to the destination path.
+     * If the destination path already exists, it is replaced. Otherwise a new file is created.
+     *
+     * Adapted from: https://github.com/Microsoft/vsts-task-lib/blob/master/node/task.ts
+     * @param sourcePath      Path to copy from
+     * @param destinationPath Path to copy to
+     */
     public static copyFile(sourcePath:string, destinationPath:string):void {
         shell.cp('-f', sourcePath, destinationPath);
 
@@ -13,7 +22,9 @@ export class FileSystemInteractions {
 
     /**
      * Create a directory at the specified path, including any folders in between.
-     * @param directoryPath Path to create.
+     *
+     * Copied from: https://github.com/Microsoft/vsts-task-lib/blob/master/node/task.ts
+     * @param directoryPath Path to create
      */
     public static createDirectory(directoryPath:string):void {
         // build a stack of directories to create
@@ -72,7 +83,14 @@ export class FileSystemInteractions {
         }
     }
 
-    public static checkShell(cmd: string, continueOnError?: boolean) {
+    /**
+     * Checks that no errors were produced by the previous command.
+     *
+     * Copied from: https://github.com/Microsoft/vsts-task-lib/blob/master/node/task.ts
+     * @param cmd             Command issued
+     * @param continueOnError Do not throw an exception if an error was produced
+     */
+    private static checkShell(cmd: string, continueOnError?: boolean) {
         var se = shell.error();
 
         if (se) {

Tests/L0/Maven/_suite.ts

@@ -8,6 +8,7 @@ import path = require('path');
 import fs = require('fs');
 import url = require('url');
 import {Url} from 'url';
+import shell = require('shelljs');
 
 import {ToolRunner} from 'vsts-task-lib/toolrunner';
 import tr = require('../../lib/vsts-task-lib/toolRunner');
@@ -23,6 +24,8 @@ import {SonarQubeMetrics} from '../../../Tasks/Maven/CodeAnalysis/SonarQube/metr
 import {SonarQubeMeasurementUnit} from '../../../Tasks/Maven/CodeAnalysis/SonarQube/metrics';
 import {MockSonarQubeServer} from './server-mock';
 
+import {FileSystemInteractions} from '../../../Tasks/Maven/CodeAnalysis/Common/FileSystemInteractions';
+
 import http = require('http');
 import {IncomingMessage} from 'http';
 
@@ -97,12 +100,7 @@ function setupMockResponsesForPaths(responseObject: any, paths: string[]) { // C
 
 // Create temp dirs for mavencodeanalysis tests to save into
 function createTempDirsForCodeAnalysisTests(): void {
-    var testTempDir: string = path.join(__dirname, '_temp');
-    var caTempDir: string = path.join(testTempDir, '.codeAnalysis');
-
-    if (!fs.existsSync(testTempDir)) {
-        fs.mkdirSync(testTempDir);
-    }
+    var caTempDir: string = path.join(createTempDir(), '.codeAnalysis');
 
     if (!fs.existsSync(caTempDir)) {
         fs.mkdirSync(caTempDir);
@@ -111,16 +109,21 @@ function createTempDirsForCodeAnalysisTests(): void {
 
 // Create temp dirs for mavencodeanalysis tests to save into
 function createTempDirsForSonarQubeTests(): void {
+    var sqTempDir: string = path.join(createTempDir(), '.sqAnalysis');
+
+    if (!fs.existsSync(sqTempDir)) {
+        fs.mkdirSync(sqTempDir);
+    }
+}
+
+function createTempDir():string {
     var testTempDir: string = path.join(__dirname, '_temp');
-    var sqTempDir: string = path.join(testTempDir, '.sqAnalysis');
 
     if (!fs.existsSync(testTempDir)) {
         fs.mkdirSync(testTempDir);
     }
 
-    if (!fs.existsSync(sqTempDir)) {
-        fs.mkdirSync(sqTempDir);
-    }
+    return testTempDir;
 }
 
 function captureStream(stream):{unhook():void, captured():string} {
@@ -1968,4 +1971,226 @@ describe('Maven Suite', function () {
                 assert(taskResult == 0 /* TaskResult.Failed == 0 */, 'Task should not have failed.');
             });
     });
+
+    /* Standalone Code Analysis unit tests */
+
+    it('Code Analysis common - createDirectory correctly creates new dir', () => {
+        // Arrange
+        var testStgDir: string = path.join(__dirname, '_temp');
+
+        if (!fs.existsSync(testStgDir)) {
+            fs.mkdirSync(testStgDir);
+        }
+
+        var newFolder1 = path.join(testStgDir, 'fish');
+
+        // Act
+        FileSystemInteractions.createDirectory(newFolder1);
+
+        // Assert
+        assert(fs.existsSync(newFolder1), `Expected folder to have been created: ${newFolder1}`);
+
+        deleteFolderRecursive(testStgDir);
+    });
+
+    it('Code Analysis common - createDirectory correctly creates new dir and 1 directory in between', () => {
+        // Arrange
+        var testStgDir: string = path.join(__dirname, '_temp');
+        var newFolder1 = path.join(testStgDir, 'fish');
+
+        if (!fs.existsSync(testStgDir)) {
+            fs.mkdirSync(testStgDir);
+        }
+        if (!fs.existsSync(newFolder1)) {
+            fs.mkdirSync(newFolder1);
+        }
+
+        var newFolder2 = path.join(testStgDir, 'fish', 'and');
+        var newFolder3 = path.join(testStgDir, 'fish', 'and', 'chips');
+
+        // Act
+        FileSystemInteractions.createDirectory(newFolder3);
+
+        // Assert
+        assert(fs.existsSync(newFolder1), `Expected folder to have been created: ${newFolder2}`);
+        assert(fs.existsSync(newFolder1), `Expected folder to have been created: ${newFolder3}`);
+
+        deleteFolderRecursive(testStgDir);
+    });
+
+    it('Code Analysis common - createDirectory correctly creates new dir and 2 directories in between', () => {
+        // Arrange
+        var testStgDir: string = path.join(__dirname, '_temp');
+        var newFolder1 = path.join(testStgDir, 'fish');
+
+        if (!fs.existsSync(testStgDir)) {
+            fs.mkdirSync(testStgDir);
+        }
+        if (!fs.existsSync(newFolder1)) {
+            fs.mkdirSync(newFolder1);
+        }
+
+        var newFolder2 = path.join(testStgDir, 'fish', 'and');
+        var newFolder3 = path.join(testStgDir, 'fish', 'and', 'chips');
+
+        // Act
+        FileSystemInteractions.createDirectory(newFolder3);
+
+        // Assert
+        assert(fs.existsSync(newFolder1), `Expected folder to have been created: ${newFolder2}`);
+        assert(fs.existsSync(newFolder1), `Expected folder to have been created: ${newFolder3}`);
+
+        deleteFolderRecursive(testStgDir);
+    });
+
+    it('Code Analysis common - createDirectory correctly creates new dir and all directories in between', () => {
+        // Arrange
+        var testStgDir: string = path.join(__dirname, '_temp');
+
+        if (!fs.existsSync(testStgDir)) {
+            fs.mkdirSync(testStgDir);
+        }
+
+        var newFolder1 = path.join(testStgDir, 'fish');
+        var newFolder2 = path.join(testStgDir, 'fish', 'and');
+        var newFolder3 = path.join(testStgDir, 'fish', 'and', 'chips');
+
+        // Act
+        FileSystemInteractions.createDirectory(newFolder3);
+
+        // Assert
+        assert(fs.existsSync(newFolder1), `Expected folder to have been created: ${newFolder1}`);
+        assert(fs.existsSync(newFolder1), `Expected folder to have been created: ${newFolder2}`);
+        assert(fs.existsSync(newFolder1), `Expected folder to have been created: ${newFolder3}`);
+
+        deleteFolderRecursive(testStgDir);
+    });
+
+    it('Code Analysis common - createDirectory correctly creates new dir and all directories in between (repeating dir names)', () => {
+        // Arrange
+        var testStgDir: string = path.join(__dirname, '_temp');
+
+        if (!fs.existsSync(testStgDir)) {
+            fs.mkdirSync(testStgDir);
+        }
+
+        var newFolder1 = path.join(testStgDir, 'fish');
+        var newFolder2 = path.join(testStgDir, 'fish', 'and');
+        var newFolder3 = path.join(testStgDir, 'fish', 'and', 'fish');
+
+        // Act
+        FileSystemInteractions.createDirectory(newFolder3);
+
+        // Assert
+        assert(fs.existsSync(newFolder1), `Expected folder to have been created: ${newFolder1}`);
+        assert(fs.existsSync(newFolder1), `Expected folder to have been created: ${newFolder2}`);
+        assert(fs.existsSync(newFolder1), `Expected folder to have been created: ${newFolder3}`);
+
+        deleteFolderRecursive(testStgDir);
+    });
+
+    // Copied from: https://github.com/Microsoft/vsts-task-lib/blob/master/node/test/dirtests.ts
+    it('Code Analysis common - createDirectory fails with illegal chars', function (done) {
+        this.timeout(1000);
+
+        var testPath = path.join(createTempDir(), 'mkdir\0');
+        var worked: boolean = false;
+        try {
+            FileSystemInteractions.createDirectory(testPath);
+            worked = true;
+        }
+        catch (err) {
+            // asserting failure
+            assert(!shell.test('-d', testPath), 'directory should not be created');
+        }
+
+        assert(!worked, 'mkdirP with illegal chars should have not have worked');
+
+        done();
+    });
+
+    // Copied from: https://github.com/Microsoft/vsts-task-lib/blob/master/node/test/dirtests.ts
+    it('Code Analysis common - createDirectory fails with null path', function (done) {
+        this.timeout(1000);
+
+        var worked: boolean = false;
+        try {
+            FileSystemInteractions.createDirectory(null);
+            worked = true;
+        }
+        catch (err) { }
+
+        assert(!worked, 'mkdirP with null should have not have worked');
+
+        done();
+    });
+
+    // Copied from: https://github.com/Microsoft/vsts-task-lib/blob/master/node/test/dirtests.ts
+    it('Code Analysis common - createDirectory fails with empty path', function (done) {
+        this.timeout(1000);
+
+        var worked: boolean = false;
+        try {
+            FileSystemInteractions.createDirectory('');
+            worked = true;
+        }
+        catch (err) { }
+
+        assert(!worked, 'mkdirP with empty string should have not have worked');
+
+        done();
+    });
+
+    // Copied from: https://github.com/Microsoft/vsts-task-lib/blob/master/node/test/dirtests.ts
+    it('Code Analysis common - createDirectory fails with conflicting file path', (done: MochaDone) => {
+        this.timeout(1000);
+
+        let testPath = path.join(createTempDir(), 'mkdirP_conflicting_file_path');
+        shell.mkdir('-p', createTempDir());
+        fs.writeFileSync(testPath, '');
+        let worked: boolean = false;
+        try {
+            FileSystemInteractions.createDirectory(testPath);
+            worked = true;
+        }
+        catch (err) { }
+
+        assert(!worked, 'mkdirP with conflicting file path should not have worked');
+
+        done();
+    });
+
+    // Copied from: https://github.com/Microsoft/vsts-task-lib/blob/master/node/test/dirtests.ts
+    it('Code Analysis common - createDirectory fails with conflicting parent file path', (done: MochaDone) => {
+        this.timeout(1000);
+
+        let testPath = path.join(createTempDir(), 'mkdirP_conflicting_parent_file_path', 'dir');
+        shell.mkdir('-p', createTempDir());
+        fs.writeFileSync(path.dirname(testPath), '');
+        let worked: boolean = false;
+        try {
+            FileSystemInteractions.createDirectory(testPath);
+            worked = true;
+        }
+        catch (err) { }
+
+        assert(!worked, 'mkdirP with conflicting file path should not have worked');
+
+        done();
+    });
+
+    // Copied from: https://github.com/Microsoft/vsts-task-lib/blob/master/node/test/dirtests.ts
+    it('Code Analysis common - createDirectory no-ops if mkdirP directory exists', (done: MochaDone) => {
+        this.timeout(1000);
+
+        let testPath = path.join(createTempDir(), 'mkdirP_dir_exists');
+        shell.mkdir('-p', createTempDir());
+        fs.mkdirSync(testPath);
+
+        FileSystemInteractions.createDirectory(testPath); // should not throw
+
+        done();
+    });
+
+
 });