From 6b763a3349785fcff021bdd49023305ec7deceb4 Mon Sep 17 00:00:00 2001 From: Emmanuel Ola <54866720+eoola@users.noreply.github.com> Date: Fri, 19 Nov 2021 12:23:01 -0500 Subject: [PATCH 1/7] put quotes around the absolute path (#1845) added quotes to wrap the path to summary.txt to ensure that paths containing spaces are read properly Co-authored-by: Matt Wynne Co-authored-by: Blaise Pabon Co-authored-by: Dane Parchment --- features/formatter_paths.feature | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/features/formatter_paths.feature b/features/formatter_paths.feature index e9bb7cb6d..ee4b6318e 100644 --- a/features/formatter_paths.feature +++ b/features/formatter_paths.feature @@ -3,15 +3,15 @@ Feature: Formatter Paths Background: Given a file named "features/a.feature" with: """ - Feature: some feature - Scenario: some scenario - Given a passing step +Feature: some feature + Scenario: some scenario + Given a passing step """ And a file named "features/step_definitions/cucumber_steps.js" with: """ - const {Given} = require('@cucumber/cucumber') + const {Given} = require('@cucumber/cucumber') - Given(/^a passing step$/, function() {}) + Given(/^a passing step$/, function() {}) """ Scenario: Relative path @@ -23,9 +23,10 @@ Feature: Formatter Paths """ + @wip Scenario: Absolute path Given "{{{tmpDir}}}" is an absolute path - When I run cucumber-js with `-f summary:{{{tmpDir}}}/summary.txt` + When I run cucumber-js with `-f summary:"{{{tmpDir}}}/summary.txt"` Then the file "{{{tmpDir}}}/summary.txt" has the text: """ 1 scenario (1 passed) From e40e94dfccf292740cc00870106b94e119563401 Mon Sep 17 00:00:00 2001 From: Emmanuel Ola <54866720+eoola@users.noreply.github.com> Date: Fri, 3 Dec 2021 11:32:50 -0500 Subject: [PATCH 2/7] fix indentations in feature file --- features/formatter_paths.feature | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/features/formatter_paths.feature b/features/formatter_paths.feature index ee4b6318e..738edbbde 100644 --- a/features/formatter_paths.feature +++ b/features/formatter_paths.feature @@ -3,15 +3,15 @@ Feature: Formatter Paths Background: Given a file named "features/a.feature" with: """ -Feature: some feature - Scenario: some scenario - Given a passing step + Feature: some feature + Scenario: some scenario + Given a passing step """ And a file named "features/step_definitions/cucumber_steps.js" with: """ - const {Given} = require('@cucumber/cucumber') + const {Given} = require('@cucumber/cucumber') - Given(/^a passing step$/, function() {}) + Given(/^a passing step$/, function() {}) """ Scenario: Relative path From 45df108e42766cf9357f4dc5a6861418479a48aa Mon Sep 17 00:00:00 2001 From: Emmanuel Ola <54866720+eoola@users.noreply.github.com> Date: Fri, 3 Dec 2021 12:30:10 -0500 Subject: [PATCH 3/7] fixed the bug but needs unit testing --- src/cli/configuration_builder.ts | 4 +++- src/cli/index.ts | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/cli/configuration_builder.ts b/src/cli/configuration_builder.ts index 384a3da67..b499c272a 100644 --- a/src/cli/configuration_builder.ts +++ b/src/cli/configuration_builder.ts @@ -177,7 +177,9 @@ export default class ConfigurationBuilder { getFormats(): IConfigurationFormat[] { const mapping: { [key: string]: string } = { '': 'progress' } this.options.format.forEach((format) => { - const [type, outputTo] = OptionSplitter.split(format) + const [type, outputToRaw] = OptionSplitter.split(format) + //TODO: test this + const outputTo = outputToRaw.replace(/"/g, "") mapping[outputTo] = type }) if (this.isPublishing()) { diff --git a/src/cli/index.ts b/src/cli/index.ts index bd5488e60..ef825b3d4 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -112,7 +112,8 @@ export default class Cli { }) stream.pipe(readerStream) } else { - const fd = await fs.open(path.resolve(this.cwd, outputTo), 'w') + const resolvedPath = path.resolve(this.cwd, outputTo) + const fd = await fs.open(resolvedPath, 'w') stream = fs.createWriteStream(null, { fd }) } } From 4ded0b6d0c5f67de343422a30243b33e293bcaf1 Mon Sep 17 00:00:00 2001 From: Emmanuel Ola <54866720+eoola@users.noreply.github.com> Date: Fri, 3 Dec 2021 12:41:20 -0500 Subject: [PATCH 4/7] fixed linting --- src/cli/configuration_builder.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cli/configuration_builder.ts b/src/cli/configuration_builder.ts index b499c272a..07539f8e9 100644 --- a/src/cli/configuration_builder.ts +++ b/src/cli/configuration_builder.ts @@ -178,8 +178,8 @@ export default class ConfigurationBuilder { const mapping: { [key: string]: string } = { '': 'progress' } this.options.format.forEach((format) => { const [type, outputToRaw] = OptionSplitter.split(format) - //TODO: test this - const outputTo = outputToRaw.replace(/"/g, "") + // TODO: test this + const outputTo = outputToRaw.replace(/"/g, '') mapping[outputTo] = type }) if (this.isPublishing()) { From af7212abea41f02bd2bf6a646cc948b05f0c1966 Mon Sep 17 00:00:00 2001 From: Emmanuel Ola <54866720+eoola@users.noreply.github.com> Date: Fri, 10 Dec 2021 11:55:36 -0500 Subject: [PATCH 5/7] adds unit testing for handling paths with quotes --- src/cli/configuration_builder.ts | 1 - src/cli/configuration_builder_spec.ts | 21 +++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/cli/configuration_builder.ts b/src/cli/configuration_builder.ts index 07539f8e9..8e6ebe245 100644 --- a/src/cli/configuration_builder.ts +++ b/src/cli/configuration_builder.ts @@ -178,7 +178,6 @@ export default class ConfigurationBuilder { const mapping: { [key: string]: string } = { '': 'progress' } this.options.format.forEach((format) => { const [type, outputToRaw] = OptionSplitter.split(format) - // TODO: test this const outputTo = outputToRaw.replace(/"/g, '') mapping[outputTo] = type }) diff --git a/src/cli/configuration_builder_spec.ts b/src/cli/configuration_builder_spec.ts index b2b8a0111..cbc7a4671 100644 --- a/src/cli/configuration_builder_spec.ts +++ b/src/cli/configuration_builder_spec.ts @@ -316,6 +316,27 @@ describe('Configuration', () => { ]) }) + it('splits absolute paths with spaces in them', async function () { + // Arrange + const cwd = await buildTestWorkingDirectory() + const argv = baseArgv.concat([ + '-f', + '/custom/formatter:"/formatter directory/output.txt"', + ]) + + // Act + const { formats } = await ConfigurationBuilder.build({ argv, cwd }) + + // Assert + expect(formats).to.eql([ + { outputTo: '', type: 'progress' }, + { + outputTo: '/formatter directory/output.txt', + type: '/custom/formatter', + }, + ]) + }) + it('splits absolute windows paths', async function () { // Arrange const cwd = await buildTestWorkingDirectory() From c3862ef3b089c90fedee61c7599cd7888de40174 Mon Sep 17 00:00:00 2001 From: Emmanuel Ola <54866720+eoola@users.noreply.github.com> Date: Fri, 10 Dec 2021 12:36:26 -0500 Subject: [PATCH 6/7] adds the fix to option splitter files --- src/cli/option_splitter.ts | 2 ++ src/cli/option_splitter_spec.ts | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/src/cli/option_splitter.ts b/src/cli/option_splitter.ts index 20f3eebe9..9b7371c7e 100644 --- a/src/cli/option_splitter.ts +++ b/src/cli/option_splitter.ts @@ -1,5 +1,7 @@ const OptionSplitter = { split(option: string): string[] { + option = option.replace(/"/g, '') + const parts = option.split(/([^A-Z]):(?!\\)/) const result = parts.reduce((memo: string[], part: string, i: number) => { diff --git a/src/cli/option_splitter_spec.ts b/src/cli/option_splitter_spec.ts index 19ec75c93..162f92e54 100644 --- a/src/cli/option_splitter_spec.ts +++ b/src/cli/option_splitter_spec.ts @@ -19,6 +19,11 @@ describe('OptionSplitter', () => { input: '/custom/formatter:/formatter/output.txt', output: ['/custom/formatter', '/formatter/output.txt'], }, + { + description: 'splits paths with quotes around them', + input: '/custom/formatter:"/formatter directory/output.txt"', + output: ['/custom/formatter', '/formatter directory/output.txt'], + }, { description: 'splits absolute windows paths', input: 'C:\\custom\\formatter:C:\\formatter\\output.txt', From 65ba09d81c9e4e1284b5cf239ee65e80ab40ad74 Mon Sep 17 00:00:00 2001 From: Emmanuel Ola <54866720+eoola@users.noreply.github.com> Date: Tue, 14 Dec 2021 13:45:38 -0500 Subject: [PATCH 7/7] updated changelog and removed wip tag --- CHANGELOG.md | 1 + features/formatter_paths.feature | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 669bf5bec..8ee39f172 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ Please see [CONTRIBUTING.md](https://github.com/cucumber/cucumber/blob/master/CO ## [Unreleased] ### Fixed +- Handles spaces in paths for developers working on cucumbers's own code ([#1845](https://github.com/cucumber/cucumber-js/issues/1845)) - Allows for parentheses in paths for developers working on cucumber's own code ([[#1735](https://github.com/cucumber/cucumber-js/issues/1735)]) - Smoother onboarding for Windows developers ([#1863](https://github.com/cucumber/cucumber-js/pull/1863)) diff --git a/features/formatter_paths.feature b/features/formatter_paths.feature index 738edbbde..886f7a8e3 100644 --- a/features/formatter_paths.feature +++ b/features/formatter_paths.feature @@ -23,7 +23,6 @@ Feature: Formatter Paths """ - @wip Scenario: Absolute path Given "{{{tmpDir}}}" is an absolute path When I run cucumber-js with `-f summary:"{{{tmpDir}}}/summary.txt"`