From c044b9be4d2a288dc80d3c8afd4b9acdd2e504ec Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 28 Jul 2019 16:12:36 -0400 Subject: [PATCH 1/2] Added error reporting for writeConfigurationFile failures --- .eslintrc.js | 1 + azure-pipelines.yml | 20 ++++++++++++ src/conversion/convertConfig.test.ts | 43 +++++++++++++++++--------- src/conversion/convertConfig.ts | 8 ++++- src/creation/writeConversionResults.ts | 2 +- 5 files changed, 57 insertions(+), 17 deletions(-) create mode 100644 azure-pipelines.yml diff --git a/.eslintrc.js b/.eslintrc.js index 23c49bc0d..bf301148a 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -44,6 +44,7 @@ module.exports = { "no-empty-function": 0, "no-restricted-syntax": 0, "no-param-reassign": 0, + "no-return-await": 0, "no-shadow": 0, "no-undef": 0, "no-useless-constructor": 0, diff --git a/azure-pipelines.yml b/azure-pipelines.yml new file mode 100644 index 000000000..1192e189d --- /dev/null +++ b/azure-pipelines.yml @@ -0,0 +1,20 @@ +trigger: + - master + +jobs: + - job: primary_code_validation_and_tests + displayName: Build + + pool: + vmImage: "Ubuntu-16.04" + + steps: + - task: NodeTool@0 + inputs: + versionSpec: 12 + displayName: "Install Node.js 12" + + - script: yarn --ignore-engines --frozen-lockfile + + - script: yarn typecheck + displayName: "Typecheck all packages" diff --git a/src/conversion/convertConfig.test.ts b/src/conversion/convertConfig.test.ts index ddb9735d7..92a9eb14f 100644 --- a/src/conversion/convertConfig.test.ts +++ b/src/conversion/convertConfig.test.ts @@ -2,10 +2,18 @@ import { ResultStatus, FailedResult, SucceededDataResult } from "../types"; import { convertConfig, ConvertConfigDependencies } from "./convertConfig"; import { OriginalConfigurations } from "../input/findOriginalConfigurations"; +const stubSettings = { + config: "./eslintrc.js", +}; + const createStubDependencies = ( - overrides: Pick, + overrides: Partial = {}, ): ConvertConfigDependencies => ({ convertRules: jest.fn(), + findOriginalConfigurations: jest.fn().mockResolvedValue({ + data: createStubOriginalConfigurationsData(), + status: ResultStatus.Succeeded, + }), reportConversionResults: jest.fn(), simplifyPackageRules: async (_configurations, data) => data, writeConversionResults: jest.fn().mockReturnValue(Promise.resolve()), @@ -29,32 +37,37 @@ describe("convertConfig", () => { const dependencies = createStubDependencies({ findOriginalConfigurations: async () => findError, }); - const settings = { - config: "./eslintrc.js", - }; // Act - const result = await convertConfig(dependencies, settings); + const result = await convertConfig(dependencies, stubSettings); // Assert expect(result).toEqual(findError); }); - it("returns a successful result when finding the original configurations succeeds", async () => { + it("returns the failure result when writing to the configuration file fails", async () => { // Arrange - const findSuccess: SucceededDataResult = { - data: createStubOriginalConfigurationsData(), - status: ResultStatus.Succeeded, - }; + const fileWriteError = new Error(); const dependencies = createStubDependencies({ - findOriginalConfigurations: async () => findSuccess, + writeConversionResults: jest.fn().mockResolvedValueOnce(fileWriteError), }); - const settings = { - config: "./eslintrc.js", - }; // Act - const result = await convertConfig(dependencies, settings); + const result = await convertConfig(dependencies, stubSettings); + + // Assert + expect(result).toEqual({ + errors: [fileWriteError], + status: ResultStatus.Failed, + }); + }); + + it("returns a successful result when finding the original configurations succeeds", async () => { + // Arrange + const dependencies = createStubDependencies(); + + // Act + const result = await convertConfig(dependencies, stubSettings); // Assert expect(result).toEqual({ diff --git a/src/conversion/convertConfig.ts b/src/conversion/convertConfig.ts index 8091c965c..a6308d039 100644 --- a/src/conversion/convertConfig.ts +++ b/src/conversion/convertConfig.ts @@ -43,11 +43,17 @@ export const convertConfig = async ( }; // 4. The simplified configuration is written to the output config file - await dependencies.writeConversionResults( + const fileWriteError = await dependencies.writeConversionResults( settings.config, simplifiedConfiguration, originalConfigurations.data, ); + if (fileWriteError !== undefined) { + return { + errors: [fileWriteError], + status: ResultStatus.Failed, + }; + } // 5. A summary of the results is printed to the user's console dependencies.reportConversionResults(simplifiedConfiguration); diff --git a/src/creation/writeConversionResults.ts b/src/creation/writeConversionResults.ts index 4aeb04361..49be3ebd7 100644 --- a/src/creation/writeConversionResults.ts +++ b/src/creation/writeConversionResults.ts @@ -33,5 +33,5 @@ export const writeConversionResults = async ( rules: formatConvertedRules(ruleConversionResults, originalConfigurations.tslint), }; - await dependencies.fileSystem.writeFile(outputPath, formatOutput(outputPath, output)); + return await dependencies.fileSystem.writeFile(outputPath, formatOutput(outputPath, output)); }; From cca5fd85a2d238655a4226857d8a4fb6be06a65b Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 28 Jul 2019 16:16:38 -0400 Subject: [PATCH 2/2] Delete azure-pipelines.yml --- azure-pipelines.yml | 20 -------------------- 1 file changed, 20 deletions(-) delete mode 100644 azure-pipelines.yml diff --git a/azure-pipelines.yml b/azure-pipelines.yml deleted file mode 100644 index 1192e189d..000000000 --- a/azure-pipelines.yml +++ /dev/null @@ -1,20 +0,0 @@ -trigger: - - master - -jobs: - - job: primary_code_validation_and_tests - displayName: Build - - pool: - vmImage: "Ubuntu-16.04" - - steps: - - task: NodeTool@0 - inputs: - versionSpec: 12 - displayName: "Install Node.js 12" - - - script: yarn --ignore-engines --frozen-lockfile - - - script: yarn typecheck - displayName: "Typecheck all packages"