From 807e57e77bd47d25dadd8611e96de7478d66adbb Mon Sep 17 00:00:00 2001 From: "david.wuerfel" Date: Sun, 6 Oct 2019 11:41:24 +0200 Subject: [PATCH 1/4] refactor (test): Extract creation of sample ts-lint rule into function --- src/rules/convertRules.test.ts | 64 ++++++++++------------------------ 1 file changed, 18 insertions(+), 46 deletions(-) diff --git a/src/rules/convertRules.test.ts b/src/rules/convertRules.test.ts index 96deede13..d74397f30 100644 --- a/src/rules/convertRules.test.ts +++ b/src/rules/convertRules.test.ts @@ -1,15 +1,19 @@ import { ConversionError } from "../errors/conversionError"; import { convertRules } from "./convertRules"; -import { TSLintRuleOptions } from "./types"; +import { TSLintRuleOptions, TSLintRuleSeverity } from "./types"; + +function createSampleTsLintRule(ruleSeverity: TSLintRuleSeverity = "error"): TSLintRuleOptions { + return { + ruleArguments: [], + ruleName: "tslint-rule-a", + ruleSeverity, + }; +} describe("convertRules", () => { it("doesn't marks a disabled rule as missing when its converter returns undefined", () => { // Arrange - const tslintRule: TSLintRuleOptions = { - ruleArguments: [], - ruleName: "tslint-rule-a", - ruleSeverity: "off", - }; + const tslintRule = createSampleTsLintRule("off"); const converters = new Map(); const mergers = new Map(); @@ -25,11 +29,7 @@ describe("convertRules", () => { it("marks an enabled rule as missing when its converter returns undefined", () => { // Arrange - const tslintRule: TSLintRuleOptions = { - ruleArguments: [], - ruleName: "tslint-rule-a", - ruleSeverity: "error", - }; + const tslintRule = createSampleTsLintRule(); const converters = new Map(); const mergers = new Map(); @@ -45,11 +45,7 @@ describe("convertRules", () => { it("marks a conversion as failed when returned a conversion error", () => { // Arrange - const tslintRule: TSLintRuleOptions = { - ruleArguments: [], - ruleName: "tslint-rule-a", - ruleSeverity: "error", - }; + const tslintRule = createSampleTsLintRule(); const conversionError = ConversionError.forRuleError(new Error(), tslintRule); const converters = new Map([[tslintRule.ruleName, () => conversionError]]); const mergers = new Map(); @@ -66,11 +62,7 @@ describe("convertRules", () => { it("marks a converted rule name as converted when a conversion has rules", () => { // Arrange - const tslintRule: TSLintRuleOptions = { - ruleArguments: [], - ruleName: "tslint-rule-a", - ruleSeverity: "error", - }; + const tslintRule = createSampleTsLintRule(); const conversionResult = { rules: [ { @@ -103,11 +95,7 @@ describe("convertRules", () => { it("reports a failure when two outputs exist for a converted rule without a merger", () => { // Arrange - const tslintRule: TSLintRuleOptions = { - ruleArguments: [], - ruleName: "tslint-rule-a", - ruleSeverity: "error", - }; + const tslintRule = createSampleTsLintRule(); const conversionResult = { rules: [ { @@ -133,11 +121,7 @@ describe("convertRules", () => { it("merges rule arguments two outputs exist for a converted rule with a merger", () => { // Arrange - const tslintRule: TSLintRuleOptions = { - ruleArguments: [], - ruleName: "tslint-rule-a", - ruleSeverity: "error", - }; + const tslintRule = createSampleTsLintRule(); const conversionResult = { rules: [ { @@ -176,11 +160,7 @@ describe("convertRules", () => { it("merges and deduplicates rule notices", () => { // Arrange - const tslintRule: TSLintRuleOptions = { - ruleArguments: [], - ruleName: "tslint-rule-a", - ruleSeverity: "error", - }; + const tslintRule = createSampleTsLintRule(); const conversionResult = { rules: [ { @@ -221,11 +201,7 @@ describe("convertRules", () => { it("merges undefined notices", () => { // Arrange - const tslintRule: TSLintRuleOptions = { - ruleArguments: [], - ruleName: "tslint-rule-a", - ruleSeverity: "error", - }; + const tslintRule = createSampleTsLintRule(); const conversionResult = { rules: [ { @@ -266,11 +242,7 @@ describe("convertRules", () => { it("marks a new plugin when a conversion has a new plugin", () => { // Arrange - const tslintRule: TSLintRuleOptions = { - ruleArguments: [], - ruleName: "tslint-rule-a", - ruleSeverity: "error", - }; + const tslintRule = createSampleTsLintRule(); const conversionResult = { plugins: ["extra-plugin"], rules: [], From 1fff23f777d56bc068a06b8fd25728e24308166d Mon Sep 17 00:00:00 2001 From: "david.wuerfel" Date: Sun, 6 Oct 2019 11:54:17 +0200 Subject: [PATCH 2/4] refactor (test): Extract the setup of the conversion environment into a method --- src/rules/convertRules.test.ts | 58 ++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 27 deletions(-) diff --git a/src/rules/convertRules.test.ts b/src/rules/convertRules.test.ts index d74397f30..44b728093 100644 --- a/src/rules/convertRules.test.ts +++ b/src/rules/convertRules.test.ts @@ -2,6 +2,16 @@ import { ConversionError } from "../errors/conversionError"; import { convertRules } from "./convertRules"; import { TSLintRuleOptions, TSLintRuleSeverity } from "./types"; +function setupConversionEnvironment( + config: { ruleSeverity: TSLintRuleSeverity } = { ruleSeverity: "error" }, +) { + const tslintRule = createSampleTsLintRule(config.ruleSeverity); + const converters = new Map(); + const mergers = new Map(); + + return { tslintRule, converters, mergers }; +} + function createSampleTsLintRule(ruleSeverity: TSLintRuleSeverity = "error"): TSLintRuleOptions { return { ruleArguments: [], @@ -13,9 +23,9 @@ function createSampleTsLintRule(ruleSeverity: TSLintRuleSeverity = "error"): TSL describe("convertRules", () => { it("doesn't marks a disabled rule as missing when its converter returns undefined", () => { // Arrange - const tslintRule = createSampleTsLintRule("off"); - const converters = new Map(); - const mergers = new Map(); + const { tslintRule, converters, mergers } = setupConversionEnvironment({ + ruleSeverity: "off", + }); // Act const { missing } = convertRules( @@ -29,9 +39,7 @@ describe("convertRules", () => { it("marks an enabled rule as missing when its converter returns undefined", () => { // Arrange - const tslintRule = createSampleTsLintRule(); - const converters = new Map(); - const mergers = new Map(); + const { tslintRule, converters, mergers } = setupConversionEnvironment(); // Act const { missing } = convertRules( @@ -45,10 +53,9 @@ describe("convertRules", () => { it("marks a conversion as failed when returned a conversion error", () => { // Arrange - const tslintRule = createSampleTsLintRule(); + const { tslintRule, converters, mergers } = setupConversionEnvironment(); const conversionError = ConversionError.forRuleError(new Error(), tslintRule); - const converters = new Map([[tslintRule.ruleName, () => conversionError]]); - const mergers = new Map(); + converters.set(tslintRule.ruleName, () => conversionError); // Act const { failed } = convertRules( @@ -62,7 +69,7 @@ describe("convertRules", () => { it("marks a converted rule name as converted when a conversion has rules", () => { // Arrange - const tslintRule = createSampleTsLintRule(); + const { tslintRule, converters, mergers } = setupConversionEnvironment(); const conversionResult = { rules: [ { @@ -70,8 +77,7 @@ describe("convertRules", () => { }, ], }; - const converters = new Map([[tslintRule.ruleName, () => conversionResult]]); - const mergers = new Map(); + converters.set(tslintRule.ruleName, () => conversionResult); // Act const { converted } = convertRules( @@ -95,7 +101,7 @@ describe("convertRules", () => { it("reports a failure when two outputs exist for a converted rule without a merger", () => { // Arrange - const tslintRule = createSampleTsLintRule(); + const { tslintRule, converters, mergers } = setupConversionEnvironment(); const conversionResult = { rules: [ { @@ -106,8 +112,7 @@ describe("convertRules", () => { }, ], }; - const converters = new Map([[tslintRule.ruleName, () => conversionResult]]); - const mergers = new Map(); + converters.set(tslintRule.ruleName, () => conversionResult); // Act const { failed } = convertRules( @@ -121,7 +126,7 @@ describe("convertRules", () => { it("merges rule arguments two outputs exist for a converted rule with a merger", () => { // Arrange - const tslintRule = createSampleTsLintRule(); + const { tslintRule, converters, mergers } = setupConversionEnvironment(); const conversionResult = { rules: [ { @@ -132,9 +137,9 @@ describe("convertRules", () => { }, ], }; + converters.set(tslintRule.ruleName, () => conversionResult); const mergedArguments = [{ merged: true }]; - const converters = new Map([[tslintRule.ruleName, () => conversionResult]]); - const mergers = new Map([[conversionResult.rules[0].ruleName, () => mergedArguments]]); + mergers.set(conversionResult.rules[0].ruleName, () => mergedArguments); // Act const { converted } = convertRules( @@ -160,7 +165,7 @@ describe("convertRules", () => { it("merges and deduplicates rule notices", () => { // Arrange - const tslintRule = createSampleTsLintRule(); + const { tslintRule, converters, mergers } = setupConversionEnvironment(); const conversionResult = { rules: [ { @@ -173,9 +178,9 @@ describe("convertRules", () => { }, ], }; + converters.set(tslintRule.ruleName, () => conversionResult); const mergedArguments = [{ merged: true }]; - const converters = new Map([[tslintRule.ruleName, () => conversionResult]]); - const mergers = new Map([[conversionResult.rules[0].ruleName, () => mergedArguments]]); + mergers.set(conversionResult.rules[0].ruleName, () => mergedArguments); // Act const { converted } = convertRules( @@ -201,7 +206,7 @@ describe("convertRules", () => { it("merges undefined notices", () => { // Arrange - const tslintRule = createSampleTsLintRule(); + const { tslintRule, converters, mergers } = setupConversionEnvironment(); const conversionResult = { rules: [ { @@ -214,9 +219,9 @@ describe("convertRules", () => { }, ], }; + converters.set(tslintRule.ruleName, () => conversionResult); const mergedArguments = [{ merged: true }]; - const converters = new Map([[tslintRule.ruleName, () => conversionResult]]); - const mergers = new Map([[conversionResult.rules[0].ruleName, () => mergedArguments]]); + mergers.set(conversionResult.rules[0].ruleName, () => mergedArguments); // Act const { converted } = convertRules( @@ -242,13 +247,12 @@ describe("convertRules", () => { it("marks a new plugin when a conversion has a new plugin", () => { // Arrange - const tslintRule = createSampleTsLintRule(); + const { tslintRule, converters, mergers } = setupConversionEnvironment(); const conversionResult = { plugins: ["extra-plugin"], rules: [], }; - const converters = new Map([[tslintRule.ruleName, () => conversionResult]]); - const mergers = new Map(); + converters.set(tslintRule.ruleName, () => conversionResult); // Act const { plugins } = convertRules( From 2426714bc5e4d468c9c19040333ada2584c6341e Mon Sep 17 00:00:00 2001 From: "david.wuerfel" Date: Sun, 6 Oct 2019 11:57:25 +0200 Subject: [PATCH 3/4] refactor (test): Add better types to Maps --- src/rules/convertRules.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/rules/convertRules.test.ts b/src/rules/convertRules.test.ts index 44b728093..47f6ac5cd 100644 --- a/src/rules/convertRules.test.ts +++ b/src/rules/convertRules.test.ts @@ -1,13 +1,15 @@ import { ConversionError } from "../errors/conversionError"; import { convertRules } from "./convertRules"; import { TSLintRuleOptions, TSLintRuleSeverity } from "./types"; +import { RuleConverter } from "./converter"; +import { RuleMerger } from "./merger"; function setupConversionEnvironment( config: { ruleSeverity: TSLintRuleSeverity } = { ruleSeverity: "error" }, ) { const tslintRule = createSampleTsLintRule(config.ruleSeverity); - const converters = new Map(); - const mergers = new Map(); + const converters = new Map(); + const mergers = new Map(); return { tslintRule, converters, mergers }; } From 17963b5ea54f81f149d0d12b66aea7409d8ebeb1 Mon Sep 17 00:00:00 2001 From: "david.wuerfel" Date: Sun, 6 Oct 2019 12:29:22 +0200 Subject: [PATCH 4/4] refactor (test): Extract parametrized converter and merger creation into method --- src/rules/convertRules.test.ts | 112 +++++++++++++++++++++------------ 1 file changed, 72 insertions(+), 40 deletions(-) diff --git a/src/rules/convertRules.test.ts b/src/rules/convertRules.test.ts index 47f6ac5cd..d542a407a 100644 --- a/src/rules/convertRules.test.ts +++ b/src/rules/convertRules.test.ts @@ -1,27 +1,9 @@ import { ConversionError } from "../errors/conversionError"; import { convertRules } from "./convertRules"; import { TSLintRuleOptions, TSLintRuleSeverity } from "./types"; -import { RuleConverter } from "./converter"; +import { RuleConverter, ConversionResult } from "./converter"; import { RuleMerger } from "./merger"; -function setupConversionEnvironment( - config: { ruleSeverity: TSLintRuleSeverity } = { ruleSeverity: "error" }, -) { - const tslintRule = createSampleTsLintRule(config.ruleSeverity); - const converters = new Map(); - const mergers = new Map(); - - return { tslintRule, converters, mergers }; -} - -function createSampleTsLintRule(ruleSeverity: TSLintRuleSeverity = "error"): TSLintRuleOptions { - return { - ruleArguments: [], - ruleName: "tslint-rule-a", - ruleSeverity, - }; -} - describe("convertRules", () => { it("doesn't marks a disabled rule as missing when its converter returns undefined", () => { // Arrange @@ -71,7 +53,6 @@ describe("convertRules", () => { it("marks a converted rule name as converted when a conversion has rules", () => { // Arrange - const { tslintRule, converters, mergers } = setupConversionEnvironment(); const conversionResult = { rules: [ { @@ -79,7 +60,9 @@ describe("convertRules", () => { }, ], }; - converters.set(tslintRule.ruleName, () => conversionResult); + const { tslintRule, converters, mergers } = setupConversionEnvironment({ + conversionResult, + }); // Act const { converted } = convertRules( @@ -103,7 +86,6 @@ describe("convertRules", () => { it("reports a failure when two outputs exist for a converted rule without a merger", () => { // Arrange - const { tslintRule, converters, mergers } = setupConversionEnvironment(); const conversionResult = { rules: [ { @@ -114,7 +96,9 @@ describe("convertRules", () => { }, ], }; - converters.set(tslintRule.ruleName, () => conversionResult); + const { tslintRule, converters, mergers } = setupConversionEnvironment({ + conversionResult, + }); // Act const { failed } = convertRules( @@ -128,7 +112,6 @@ describe("convertRules", () => { it("merges rule arguments two outputs exist for a converted rule with a merger", () => { // Arrange - const { tslintRule, converters, mergers } = setupConversionEnvironment(); const conversionResult = { rules: [ { @@ -139,9 +122,10 @@ describe("convertRules", () => { }, ], }; - converters.set(tslintRule.ruleName, () => conversionResult); - const mergedArguments = [{ merged: true }]; - mergers.set(conversionResult.rules[0].ruleName, () => mergedArguments); + const { tslintRule, converters, mergers } = setupConversionEnvironment({ + conversionResult, + ruleToMerge: conversionResult.rules[0].ruleName, + }); // Act const { converted } = convertRules( @@ -155,7 +139,7 @@ describe("convertRules", () => { [ "eslint-rule-a", { - ruleArguments: mergedArguments, + ruleArguments: [{ merged: true }], ruleName: "eslint-rule-a", ruleSeverity: "error", notices: [], @@ -167,7 +151,6 @@ describe("convertRules", () => { it("merges and deduplicates rule notices", () => { // Arrange - const { tslintRule, converters, mergers } = setupConversionEnvironment(); const conversionResult = { rules: [ { @@ -180,9 +163,10 @@ describe("convertRules", () => { }, ], }; - converters.set(tslintRule.ruleName, () => conversionResult); - const mergedArguments = [{ merged: true }]; - mergers.set(conversionResult.rules[0].ruleName, () => mergedArguments); + const { tslintRule, converters, mergers } = setupConversionEnvironment({ + conversionResult, + ruleToMerge: conversionResult.rules[0].ruleName, + }); // Act const { converted } = convertRules( @@ -196,7 +180,7 @@ describe("convertRules", () => { [ "eslint-rule-a", { - ruleArguments: mergedArguments, + ruleArguments: [{ merged: true }], ruleName: "eslint-rule-a", ruleSeverity: "error", notices: ["notice-1", "notice-2"], @@ -208,7 +192,6 @@ describe("convertRules", () => { it("merges undefined notices", () => { // Arrange - const { tslintRule, converters, mergers } = setupConversionEnvironment(); const conversionResult = { rules: [ { @@ -221,9 +204,10 @@ describe("convertRules", () => { }, ], }; - converters.set(tslintRule.ruleName, () => conversionResult); - const mergedArguments = [{ merged: true }]; - mergers.set(conversionResult.rules[0].ruleName, () => mergedArguments); + const { tslintRule, converters, mergers } = setupConversionEnvironment({ + conversionResult, + ruleToMerge: conversionResult.rules[0].ruleName, + }); // Act const { converted } = convertRules( @@ -237,7 +221,7 @@ describe("convertRules", () => { [ "eslint-rule-a", { - ruleArguments: mergedArguments, + ruleArguments: [{ merged: true }], ruleName: "eslint-rule-a", ruleSeverity: "error", notices: [], @@ -249,12 +233,13 @@ describe("convertRules", () => { it("marks a new plugin when a conversion has a new plugin", () => { // Arrange - const { tslintRule, converters, mergers } = setupConversionEnvironment(); const conversionResult = { plugins: ["extra-plugin"], rules: [], }; - converters.set(tslintRule.ruleName, () => conversionResult); + const { tslintRule, converters, mergers } = setupConversionEnvironment({ + conversionResult, + }); // Act const { plugins } = convertRules( @@ -266,3 +251,50 @@ describe("convertRules", () => { expect(plugins).toEqual(new Set(["extra-plugin"])); }); }); + +function setupConversionEnvironment( + config: { + ruleSeverity?: TSLintRuleSeverity; + conversionResult?: ConversionResult; + ruleToMerge?: string; + } = {}, +) { + const { ruleSeverity, conversionResult, ruleToMerge } = config; + + const tslintRule = createSampleTsLintRule(ruleSeverity); + const converters = createConverters(tslintRule, conversionResult); + const mergers = createMergers(ruleToMerge); + + return { tslintRule, converters, mergers }; +} + +function createSampleTsLintRule(ruleSeverity: TSLintRuleSeverity = "error"): TSLintRuleOptions { + return { + ruleArguments: [], + ruleName: "tslint-rule-a", + ruleSeverity, + }; +} + +function createConverters( + tslintRule: TSLintRuleOptions, + conversionResult?: ConversionResult, +): Map { + const converters = new Map(); + + if (conversionResult !== undefined) { + converters.set(tslintRule.ruleName, () => conversionResult); + } + + return converters; +} + +function createMergers(ruleToMerge?: string): Map { + const mergers = new Map(); + + if (ruleToMerge !== undefined) { + mergers.set(ruleToMerge, () => [{ merged: true }]); + } + + return mergers; +}