From de8407c3213eac8e6cee49b634d50cf47aa5a965 Mon Sep 17 00:00:00 2001 From: Edward Bebbington Date: Wed, 23 Sep 2020 20:53:20 +0100 Subject: [PATCH] [issue-#5-only] Got only usage working, temp changed output format --- mod.ts | 98 ++++++++++++++++------------------ src/interfaces.ts | 2 +- src/test_case.ts | 50 +++++++++++------ tests/integration/only_test.ts | 1 + 4 files changed, 81 insertions(+), 70 deletions(-) diff --git a/mod.ts b/mod.ts index 89b93b54..a26a758d 100644 --- a/mod.ts +++ b/mod.ts @@ -232,19 +232,19 @@ export class RhumRunner { * @param name - The name * @param cb - The callback */ - public only(name: string, cb: Function): void { - // FIXME(edward) Any test suite within a plan will a ctually hit this block, if there was a test suite before it. It should not hit this block - if (this.passed_in_test_plan && this.passed_in_test_suite) { // is a test case being skipped + public only(name: string, cb: () => void): void { + if (this.passed_in_test_plan && this.passed_in_test_suite) { // is a test case being skipped, so do the same thing we would do in `testCase` this.plan.suites[this.passed_in_test_suite].cases!.push({ name, new_name: this.formatTestCaseName(name), testFn: cb, only: true }); - } else if (this.passed_in_test_plan) { // is a test suite being skipped + } else if (this.passed_in_test_plan) { // is a test suite being skipped, so do the same thing we could do in `testSuite` this.passed_in_test_suite = name; this.plan.suites![name] = {cases: [], only: true}; cb(); + this.passed_in_test_suite = "" // At the end of the suite, remove the name ready for a new suite. The reason for this is mainly when using `only`, so say when a 2nd suite uses `only`, it can flow through the logic properly } } @@ -400,6 +400,7 @@ export class RhumRunner { this.passed_in_test_suite = name; this.plan.suites![name] = { cases: [], only: false }; testCases(); + this.passed_in_test_suite = "" // At the end of the suite, remove the name ready for a new suite. The reason for this is mainly when using `only`, so say when a 2nd suite uses `only`, it can flow through the logic properly } /** @@ -412,24 +413,33 @@ export class RhumRunner { * Rhum.run(); */ public run(): void { - const onlySuite: string|undefined = Object.keys(this.plan.suites).filter(suiteName => this.plan.suites[suiteName].only === true)[0] - const onlyCase: string|undefined = Object.keys(this.plan.suites).filter(suiteName => this.plan.suites[suiteName].cases!.filter(c => c.only === true).length > 0)[0] - if (onlySuite) { - this.plan.suites = { - [onlySuite]: this.plan.suites[onlySuite] - }; - } else if (onlyCase) { - // Select that test case - this.plan.suites = { - [onlyCase]: this.plan.suites[onlyCase] + // + // Validation for using `only`, mainly edge cases + // + const suitesWithOnly = Object.keys(this.plan.suites).filter(suiteName => this.plan.suites[suiteName].only === true) + const casesWithOnly: string[] = [] + for (const suite in this.plan.suites) { + const onlyCases = this.plan.suites[suite].cases!.filter(c => c.only === true); + if (onlyCases.length) { + onlyCases.forEach(c => { + casesWithOnly.push(c.name) + }) } - this.plan.suites[onlyCase].cases = this.plan.suites[onlyCase].cases!.filter(c => c.only === true) - // We need to re-format the name to make it display the plan and suite - const tmpTestPlanInProgress = this.test_suite_in_progress - this.test_plan_in_progress = ""; - this.plan.suites[onlyCase].cases![0].new_name = this.formatTestCaseName(this.plan.suites[onlyCase].cases![0].name); - this.passed_in_test_suite = tmpTestPlanInProgress } + // For when a user has set a suite and test case to only.. naughty + if (suitesWithOnly.length && casesWithOnly.length) { + throw new Error("A test suite and test case have been set to only in plan `" + this.passed_in_test_plan + ". Only one is allowed") + } + // Or when a user has two suites set to only... naughty + if (casesWithOnly.length > 1) { + throw new Error("Expected one test case as `only`, but " + casesWithOnly.length + " have been defined.") + } + // Or when a user has two cases seet to only.. naughty + if (suitesWithOnly.length > 1) { + throw new Error("Expected one test suite as `only`, but " + suitesWithOnly.length + " have been defined.") + } + + // Run them const tc = new TestCase(this.plan); tc.run(); this.deconstruct(); @@ -447,44 +457,26 @@ export class RhumRunner { * Returns the new test name for outputting purposes. */ protected formatTestCaseName(name: string): string { - let newName: string; - // (ebebbington) Unfortunately, due to the CI not correctly displaying output - // (it is all over the place and just completely unreadable as - // it doesn't play well with our control characters), we need to - // display the test output differently, based on if the tests are - // being ran inside a CI or not. Nothing will change for the current - // way of doing things, but if the tests are being ran inside a CI, - // the format would be: - // test | | ... ok (2ms) - // test | | ... ok (2ms) - // Even if plans and/or suites are the same. I believe this the best - // way we can display the output - if (Deno.env.get("CI") === "true") { - newName = - `${this.passed_in_test_plan} | ${this.passed_in_test_suite} | ${name}`; - return newName; - } + // First test case for a new plan and suite if (this.test_plan_in_progress != this.passed_in_test_plan) { this.test_plan_in_progress = this.passed_in_test_plan; this.test_suite_in_progress = this.passed_in_test_suite; - newName = `${"\u0008".repeat(name.length + extraChars)}` + // strip "test " - `${" ".repeat(name.length + extraChars)}` + - `\n${this.passed_in_test_plan}` + - `\n ${this.passed_in_test_suite}` + - `\n ${name} ... `; - } else { - if (this.test_suite_in_progress != this.passed_in_test_suite) { - this.test_suite_in_progress = this.passed_in_test_suite; - newName = `${"\u0008".repeat(name.length + extraChars)}` + - ` ${this.passed_in_test_suite}` + - `${" ".repeat(name.length + extraChars)}` + - `\n ${name} ... `; - } else { - newName = `${"\u0008".repeat(name.length + extraChars)}` + - ` ${name} ... `; - } + const newName = `| ${this.passed_in_test_plan}` + + `\n | ${this.passed_in_test_suite}` + + `\n | ${name}`; + return newName; + } + + // Case for existing plan but new suite + if (this.test_suite_in_progress != this.passed_in_test_suite) { + this.test_suite_in_progress = this.passed_in_test_suite; + const newName = `| ${this.passed_in_test_suite}` + + `\n | ${name}`; + return newName; } + // Case for existing plan and suite + const newName = `| ${name}`; return newName; } diff --git a/src/interfaces.ts b/src/interfaces.ts index c6b47f08..4690e47b 100644 --- a/src/interfaces.ts +++ b/src/interfaces.ts @@ -119,8 +119,8 @@ export interface ITestSuite { * argument of Deno.test(). */ export interface ITestCase { - name: string; only: boolean; + name: string; new_name: string; testFn: () => void; } diff --git a/src/test_case.ts b/src/test_case.ts index 20467ed2..febcff33 100644 --- a/src/test_case.ts +++ b/src/test_case.ts @@ -25,6 +25,7 @@ export class TestCase { if (this.plan.hasOwnProperty("suites") === false) { return; } + Object.keys(this.plan.suites).forEach((suiteName) => { // Run cases this.plan!.suites[suiteName].cases!.forEach(async (c: ITestCase) => { @@ -59,22 +60,39 @@ export class TestCase { } }; - // (ebebbington) To stop the output of test running being horrible - // in the CI, we will only display the new name which should be - // "plan | suite " case", as opposed to the "super saiyan" - // version. This name is generated differently inside `formatTestCaseName` - // based on if the tests are being ran inside a CI job - if (Deno.env.get("CI") === "true") { - await Deno.test(c.new_name, async () => { - await hookAttachedTestFn(); - }); - } else { - await Deno.test(c.name, async () => { - Deno.stdout.writeSync(encoder.encode(c.new_name)); - await hookAttachedTestFn(); - }); + // Because lengths of test case names vary, for example: + // + // test plan + // test suite + // test case ... ok + // another test case ... ok + // + // We are going to make sure the "... ok" parts display in a nice column, + // by getting the length of the longest test case name, and ensuring each line is a consistent length + // that would match the total length of the longest test case name (plus any extra spaces), eg + // + // test plan + // test suite + // test case ... ok + // another test case ... ok + let longestCaseNameLen = 0; + for (const s in this.plan.suites) { + const len = Math.max( + ...(this.plan.suites[s].cases!.map((c) => c.name.length)), + ); + if (len > longestCaseNameLen) longestCaseNameLen = len; } - }); - }); + const numberOfExtraSpaces = longestCaseNameLen - c.name.length; // for example, it would be 0 for when it's the test with the longest case name. It's just the character difference between the current case name and longest, telling us how many spaces to add + + const isOnly = this.plan.only || this.plan.suites[suiteName].only || c.only + await Deno.test({ + name: c.new_name + " ".repeat(numberOfExtraSpaces), + ignore: isOnly === false, + async fn(): Promise { + await hookAttachedTestFn(); + } + }) + }) + }) } } diff --git a/tests/integration/only_test.ts b/tests/integration/only_test.ts index 8dfa1442..98f33f9f 100644 --- a/tests/integration/only_test.ts +++ b/tests/integration/only_test.ts @@ -23,6 +23,7 @@ Deno.test({ }); const status = await p.status(); p.close(); + console.log(new TextDecoder().decode(await p.stderrOutput())) asserts.assertEquals(status.success, true); asserts.assertEquals(status.code, 0); /**