From 2a46fd7487bf99231733117b664e5ba712488e93 Mon Sep 17 00:00:00 2001 From: David Goss Date: Wed, 4 Oct 2023 09:42:25 +0100 Subject: [PATCH 1/8] dont use OptionSplitter for order options --- src/cli/helpers.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/cli/helpers.ts b/src/cli/helpers.ts index 0571d246c..43b73ae29 100644 --- a/src/cli/helpers.ts +++ b/src/cli/helpers.ts @@ -3,7 +3,6 @@ import { EventEmitter } from 'events' import PickleFilter from '../pickle_filter' import { EventDataCollector } from '../formatter/helpers' import { doesHaveValue } from '../value_checker' -import { OptionSplitter } from '../configuration' import { Readable } from 'stream' import os from 'os' import * as messages from '@cucumber/messages' @@ -71,7 +70,7 @@ export function orderPickles( order: PickleOrder, logger: ILogger ): void { - const [type, seed] = OptionSplitter.split(order) + const [type, seed] = splitOrder(order) switch (type) { case 'defined': break @@ -91,6 +90,13 @@ export function orderPickles( } } +function splitOrder(order: string) { + if (!order.includes(':')) { + return [order, ''] + } + return order.split(':') +} + export async function emitMetaMessage( eventBroadcaster: EventEmitter, env: NodeJS.ProcessEnv From 5cdab59d0b0eade041e945f8ff8219d11d7726d7 Mon Sep 17 00:00:00 2001 From: David Goss Date: Wed, 4 Oct 2023 09:45:42 +0100 Subject: [PATCH 2/8] recast as function --- src/api/convert_configuration.ts | 4 +- src/configuration/index.ts | 2 +- src/configuration/option_splitter.ts | 57 ------------------- src/configuration/split_format_descriptor.ts | 54 ++++++++++++++++++ ...pec.ts => split_format_descriptor_spec.ts} | 4 +- 5 files changed, 59 insertions(+), 62 deletions(-) delete mode 100644 src/configuration/option_splitter.ts create mode 100644 src/configuration/split_format_descriptor.ts rename src/configuration/{option_splitter_spec.ts => split_format_descriptor_spec.ts} (98%) diff --git a/src/api/convert_configuration.ts b/src/api/convert_configuration.ts index 588f40204..8718c51fc 100644 --- a/src/api/convert_configuration.ts +++ b/src/api/convert_configuration.ts @@ -1,7 +1,7 @@ import { IConfiguration, isTruthyString, - OptionSplitter, + splitFormatDescriptor, } from '../configuration' import { IPublishConfig } from '../formatter' import { IRunConfiguration } from './types' @@ -42,7 +42,7 @@ function convertFormats( env: NodeJS.ProcessEnv ) { const splitFormats: string[][] = flatConfiguration.format.map((item) => - Array.isArray(item) ? item : OptionSplitter.split(item) + Array.isArray(item) ? item : splitFormatDescriptor(item) ) return { stdout: diff --git a/src/configuration/index.ts b/src/configuration/index.ts index 4f4bd9301..f8d91c2b8 100644 --- a/src/configuration/index.ts +++ b/src/configuration/index.ts @@ -3,5 +3,5 @@ export * from './default_configuration' export * from './from_file' export * from './helpers' export * from './merge_configurations' -export * from './option_splitter' +export * from './split_format_descriptor' export * from './types' diff --git a/src/configuration/option_splitter.ts b/src/configuration/option_splitter.ts deleted file mode 100644 index ce6863773..000000000 --- a/src/configuration/option_splitter.ts +++ /dev/null @@ -1,57 +0,0 @@ -export const OptionSplitter = { - split(option: string): string[] { - let result: string[] - let match1, match2 - - // "foo":"bar" or "foo":bar - if ((match1 = option.match(/^"([^"]*)":(.*)$/)) !== null) { - // "foo":"bar" - if ((match2 = match1[2].match(/^"([^"]*)"$/)) !== null) { - result = [match1[1], match2[1]] - } - // "foo":bar - else { - result = [match1[1], match1[2]] - } - } - // foo:"bar" - else if ((match1 = option.match(/^(.*):"([^"]*)"$/)) !== null) { - result = [match1[1], match1[2]] - } - // "foo" - else if ((match1 = option.match(/^"([^"]*)"$/)) !== null) { - result = [match1[1], ''] - } - // file://foo or file:///foo or file://C:/foo or file://C:\foo or file:///C:/foo or file:///C:\foo - else if ( - (match1 = option.match(/^(file:\/{2,3}(?:[a-zA-Z]:[/\\])?)(.*)$/)) !== - null - ) { - // file://foo:bar - if ((match2 = match1[2].match(/^([^:]*):(.*)$/)) !== null) { - result = [match1[1] + match2[1], match2[2]] - } else { - result = [option, ''] - } - } - // C:\foo or C:/foo - else if ((match1 = option.match(/^([a-zA-Z]:[/\\])(.*)$/)) !== null) { - // C:\foo:bar or C:/foo:bar - if ((match2 = match1[2].match(/^([^:]*):(.*)$/)) !== null) { - result = [match1[1] + match2[1], match2[2]] - } else { - result = [option, ''] - } - } - // foo:bar - else if ((match1 = option.match(/^([^:]*):(.*)$/)) !== null) { - result = [match1[1], match1[2]] - } - // foo - else { - result = [option, ''] - } - - return result - }, -} diff --git a/src/configuration/split_format_descriptor.ts b/src/configuration/split_format_descriptor.ts new file mode 100644 index 000000000..0843ec4b2 --- /dev/null +++ b/src/configuration/split_format_descriptor.ts @@ -0,0 +1,54 @@ +export function splitFormatDescriptor(option: string): string[] { + let result: string[] + let match1, match2 + + // "foo":"bar" or "foo":bar + if ((match1 = option.match(/^"([^"]*)":(.*)$/)) !== null) { + // "foo":"bar" + if ((match2 = match1[2].match(/^"([^"]*)"$/)) !== null) { + result = [match1[1], match2[1]] + } + // "foo":bar + else { + result = [match1[1], match1[2]] + } + } + // foo:"bar" + else if ((match1 = option.match(/^(.*):"([^"]*)"$/)) !== null) { + result = [match1[1], match1[2]] + } + // "foo" + else if ((match1 = option.match(/^"([^"]*)"$/)) !== null) { + result = [match1[1], ''] + } + // file://foo or file:///foo or file://C:/foo or file://C:\foo or file:///C:/foo or file:///C:\foo + else if ( + (match1 = option.match(/^(file:\/{2,3}(?:[a-zA-Z]:[/\\])?)(.*)$/)) !== null + ) { + // file://foo:bar + if ((match2 = match1[2].match(/^([^:]*):(.*)$/)) !== null) { + result = [match1[1] + match2[1], match2[2]] + } else { + result = [option, ''] + } + } + // C:\foo or C:/foo + else if ((match1 = option.match(/^([a-zA-Z]:[/\\])(.*)$/)) !== null) { + // C:\foo:bar or C:/foo:bar + if ((match2 = match1[2].match(/^([^:]*):(.*)$/)) !== null) { + result = [match1[1] + match2[1], match2[2]] + } else { + result = [option, ''] + } + } + // foo:bar + else if ((match1 = option.match(/^([^:]*):(.*)$/)) !== null) { + result = [match1[1], match1[2]] + } + // foo + else { + result = [option, ''] + } + + return result +} diff --git a/src/configuration/option_splitter_spec.ts b/src/configuration/split_format_descriptor_spec.ts similarity index 98% rename from src/configuration/option_splitter_spec.ts rename to src/configuration/split_format_descriptor_spec.ts index a8cdfafc3..0fd73c04e 100644 --- a/src/configuration/option_splitter_spec.ts +++ b/src/configuration/split_format_descriptor_spec.ts @@ -1,6 +1,6 @@ import { describe, it } from 'mocha' import { expect } from 'chai' -import { OptionSplitter } from './option_splitter' +import { splitFormatDescriptor } from './split_format_descriptor' describe('OptionSplitter', () => { const examples = [ @@ -182,7 +182,7 @@ describe('OptionSplitter', () => { examples.forEach(({ description, input, output }) => { it(description, () => { - expect(OptionSplitter.split(input)).to.eql(output) + expect(splitFormatDescriptor(input)).to.eql(output) }) }) }) From e5ca2968f230eb1e893c786814f106d22268947c Mon Sep 17 00:00:00 2001 From: David Goss Date: Wed, 4 Oct 2023 09:49:58 +0100 Subject: [PATCH 3/8] add logger into mix --- src/api/convert_configuration.ts | 7 +++++-- src/api/convert_configuration_spec.ts | 10 +++++++++- src/api/load_configuration.ts | 2 +- src/configuration/split_format_descriptor.ts | 7 ++++++- src/configuration/split_format_descriptor_spec.ts | 6 ++++-- 5 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/api/convert_configuration.ts b/src/api/convert_configuration.ts index 8718c51fc..93c6d60db 100644 --- a/src/api/convert_configuration.ts +++ b/src/api/convert_configuration.ts @@ -5,8 +5,10 @@ import { } from '../configuration' import { IPublishConfig } from '../formatter' import { IRunConfiguration } from './types' +import { ILogger } from '../logger' export async function convertConfiguration( + logger: ILogger, flatConfiguration: IConfiguration, env: NodeJS.ProcessEnv ): Promise { @@ -33,16 +35,17 @@ export async function convertConfiguration( strict: flatConfiguration.strict, worldParameters: flatConfiguration.worldParameters, }, - formats: convertFormats(flatConfiguration, env), + formats: convertFormats(logger, flatConfiguration, env), } } function convertFormats( + logger: ILogger, flatConfiguration: IConfiguration, env: NodeJS.ProcessEnv ) { const splitFormats: string[][] = flatConfiguration.format.map((item) => - Array.isArray(item) ? item : splitFormatDescriptor(item) + Array.isArray(item) ? item : splitFormatDescriptor(logger, item) ) return { stdout: diff --git a/src/api/convert_configuration_spec.ts b/src/api/convert_configuration_spec.ts index 4ca25d26b..156e6ef8c 100644 --- a/src/api/convert_configuration_spec.ts +++ b/src/api/convert_configuration_spec.ts @@ -2,10 +2,15 @@ import { describe, it } from 'mocha' import { expect } from 'chai' import { convertConfiguration } from './convert_configuration' import { DEFAULT_CONFIGURATION } from '../configuration' +import { FakeLogger } from '../../test/fake_logger' describe('convertConfiguration', () => { it('should convert defaults correctly', async () => { - const result = await convertConfiguration(DEFAULT_CONFIGURATION, {}) + const result = await convertConfiguration( + new FakeLogger(), + DEFAULT_CONFIGURATION, + {} + ) expect(result).to.eql({ formats: { @@ -41,6 +46,7 @@ describe('convertConfiguration', () => { it('should map multiple formatters with string notation', async () => { const result = await convertConfiguration( + new FakeLogger(), { ...DEFAULT_CONFIGURATION, format: [ @@ -66,6 +72,7 @@ describe('convertConfiguration', () => { it('should map multiple formatters with array notation', async () => { const result = await convertConfiguration( + new FakeLogger(), { ...DEFAULT_CONFIGURATION, format: [ @@ -91,6 +98,7 @@ describe('convertConfiguration', () => { it('should map formatters correctly when file:// urls are involved', async () => { const result = await convertConfiguration( + new FakeLogger(), { ...DEFAULT_CONFIGURATION, format: [ diff --git a/src/api/load_configuration.ts b/src/api/load_configuration.ts index b77d8476f..4503c3aaa 100644 --- a/src/api/load_configuration.ts +++ b/src/api/load_configuration.ts @@ -44,7 +44,7 @@ export async function loadConfiguration( ) logger.debug('Resolved configuration:', original) validateConfiguration(original, logger) - const runnable = await convertConfiguration(original, env) + const runnable = await convertConfiguration(logger, original, env) return { useConfiguration: original, runConfiguration: runnable, diff --git a/src/configuration/split_format_descriptor.ts b/src/configuration/split_format_descriptor.ts index 0843ec4b2..ba137d38f 100644 --- a/src/configuration/split_format_descriptor.ts +++ b/src/configuration/split_format_descriptor.ts @@ -1,4 +1,9 @@ -export function splitFormatDescriptor(option: string): string[] { +import { ILogger } from '../logger' + +export function splitFormatDescriptor( + logger: ILogger, + option: string +): string[] { let result: string[] let match1, match2 diff --git a/src/configuration/split_format_descriptor_spec.ts b/src/configuration/split_format_descriptor_spec.ts index 0fd73c04e..3c91e70b2 100644 --- a/src/configuration/split_format_descriptor_spec.ts +++ b/src/configuration/split_format_descriptor_spec.ts @@ -1,8 +1,9 @@ import { describe, it } from 'mocha' import { expect } from 'chai' import { splitFormatDescriptor } from './split_format_descriptor' +import { FakeLogger } from '../../test/fake_logger' -describe('OptionSplitter', () => { +describe('splitFormatDescriptor', () => { const examples = [ { description: "doesn't split when nothing to split on, adds empty string", @@ -182,7 +183,8 @@ describe('OptionSplitter', () => { examples.forEach(({ description, input, output }) => { it(description, () => { - expect(splitFormatDescriptor(input)).to.eql(output) + const logger = new FakeLogger() + expect(splitFormatDescriptor(logger, input)).to.eql(output) }) }) }) From f2f399d3db11e72e3c405f999b618ef15c09e934 Mon Sep 17 00:00:00 2001 From: David Goss Date: Fri, 6 Oct 2023 17:41:32 +0100 Subject: [PATCH 4/8] put the logging logic in place --- src/configuration/split_format_descriptor.ts | 18 +++++++++++ .../split_format_descriptor_spec.ts | 32 ++++++++++++++++++- 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/src/configuration/split_format_descriptor.ts b/src/configuration/split_format_descriptor.ts index ba137d38f..10fc805e5 100644 --- a/src/configuration/split_format_descriptor.ts +++ b/src/configuration/split_format_descriptor.ts @@ -4,6 +4,13 @@ export function splitFormatDescriptor( logger: ILogger, option: string ): string[] { + const doWarning = (result: string[]) => { + let expected = `"${result[0]}"` + if (result[1]) { + expected += `:"${result[1]}"` + } + logger.warn(`Should be ${expected}`) + } let result: string[] let match1, match2 @@ -16,11 +23,17 @@ export function splitFormatDescriptor( // "foo":bar else { result = [match1[1], match1[2]] + if (result[1].includes(':')) { + doWarning(result) + } } } // foo:"bar" else if ((match1 = option.match(/^(.*):"([^"]*)"$/)) !== null) { result = [match1[1], match1[2]] + if (result[0].includes(':')) { + doWarning(result) + } } // "foo" else if ((match1 = option.match(/^"([^"]*)"$/)) !== null) { @@ -36,6 +49,7 @@ export function splitFormatDescriptor( } else { result = [option, ''] } + doWarning(result) } // C:\foo or C:/foo else if ((match1 = option.match(/^([a-zA-Z]:[/\\])(.*)$/)) !== null) { @@ -45,10 +59,14 @@ export function splitFormatDescriptor( } else { result = [option, ''] } + doWarning(result) } // foo:bar else if ((match1 = option.match(/^([^:]*):(.*)$/)) !== null) { result = [match1[1], match1[2]] + if (option.split(':').length > 2) { + doWarning(result) + } } // foo else { diff --git a/src/configuration/split_format_descriptor_spec.ts b/src/configuration/split_format_descriptor_spec.ts index 3c91e70b2..1d956025a 100644 --- a/src/configuration/split_format_descriptor_spec.ts +++ b/src/configuration/split_format_descriptor_spec.ts @@ -24,6 +24,8 @@ describe('splitFormatDescriptor', () => { description: 'splits file URLs for absolute unix path', input: 'file:///custom/formatter:file:///formatter/output.txt', output: ['file:///custom/formatter', 'file:///formatter/output.txt'], + warning: + 'Should be "file:///custom/formatter":"file:///formatter/output.txt"', }, { description: 'splits file URLs for UNC path', @@ -33,6 +35,8 @@ describe('splitFormatDescriptor', () => { 'file://hostname/custom/formatter', 'file://hostname/formatter/output.txt', ], + warning: + 'Should be "file://hostname/custom/formatter":"file://hostname/formatter/output.txt"', }, { description: 'splits file URLs for absolute windows path', @@ -41,6 +45,8 @@ describe('splitFormatDescriptor', () => { 'file://C:\\custom\\formatter', 'file://C:\\formatter\\output.txt', ], + warning: + 'Should be "file://C:\\custom\\formatter":"file://C:\\formatter\\output.txt"', }, { description: @@ -50,6 +56,8 @@ describe('splitFormatDescriptor', () => { 'file:///C:/custom/formatter', 'file:///C:/formatter/output.txt', ], + warning: + 'Should be "file:///C:/custom/formatter":"file:///C:/formatter/output.txt"', }, { description: 'splits valid file URLs for absolute windows path', @@ -58,6 +66,8 @@ describe('splitFormatDescriptor', () => { 'file:///C:\\custom\\formatter', 'file:///C:\\formatter\\output.txt', ], + warning: + 'Should be "file:///C:\\custom\\formatter":"file:///C:\\formatter\\output.txt"', }, { description: @@ -67,6 +77,8 @@ describe('splitFormatDescriptor', () => { 'file:///C:/custom/formatter', 'file:///C:/formatter/output.txt', ], + warning: + 'Should be "file:///C:/custom/formatter":"file:///C:/formatter/output.txt"', }, { description: 'splits absolute unix paths', @@ -77,12 +89,14 @@ describe('splitFormatDescriptor', () => { description: 'splits absolute windows paths', input: 'C:\\custom\\formatter:C:\\formatter\\output.txt', output: ['C:\\custom\\formatter', 'C:\\formatter\\output.txt'], + warning: 'Should be "C:\\custom\\formatter":"C:\\formatter\\output.txt"', }, { description: 'splits absolute windows paths with "/" as directory separator', input: 'C:/custom/formatter:C:/formatter/output.txt', output: ['C:/custom/formatter', 'C:/formatter/output.txt'], + warning: 'Should be "C:/custom/formatter":"C:/formatter/output.txt"', }, { description: 'splits UNC paths', @@ -111,48 +125,56 @@ describe('splitFormatDescriptor', () => { 'does not split a single file URL for absolute unix path, adds empty string', input: 'file:///custom/formatter', output: ['file:///custom/formatter', ''], + warning: 'Should be "file:///custom/formatter"', }, { description: 'does not split a single file URL for UNC path, adds empty string', input: 'file://hostname/custom/formatter', output: ['file://hostname/custom/formatter', ''], + warning: 'Should be "file://hostname/custom/formatter"', }, { description: 'does not split a single file URL for absolute windows path, adds empty string', input: 'file://C:\\custom\\formatter', output: ['file://C:\\custom\\formatter', ''], + warning: 'Should be "file://C:\\custom\\formatter"', }, { description: 'does not split a single file URL for absolute windows path with "/" as directory separator, adds empty string', input: 'file://C:/custom/formatter', output: ['file://C:/custom/formatter', ''], + warning: 'Should be "file://C:/custom/formatter"', }, { description: 'does not split a valid single file URL for absolute windows path, adds empty string', input: 'file:///C:\\custom\\formatter', output: ['file:///C:\\custom\\formatter', ''], + warning: 'Should be "file:///C:\\custom\\formatter"', }, { description: 'does not split a valid single file URL for absolute windows path with "/" as directory separator, adds empty string', input: 'file:///C:/custom/formatter', output: ['file:///C:/custom/formatter', ''], + warning: 'Should be "file:///C:/custom/formatter"', }, { description: 'does not split a single absolute windows path, adds empty string', input: 'C:\\custom\\formatter', output: ['C:\\custom\\formatter', ''], + warning: 'Should be "C:\\custom\\formatter"', }, { description: 'does not split a single absolute windows path with "/" as directory separator, adds empty string', input: 'C:/custom/formatter', output: ['C:/custom/formatter', ''], + warning: 'Should be "C:/custom/formatter"', }, { description: 'does not split quoted values: case 1', @@ -163,11 +185,13 @@ describe('splitFormatDescriptor', () => { description: 'does not split quoted values: case 2', input: '"foo:bar":baz:qux', output: ['foo:bar', 'baz:qux'], + warning: 'Should be "foo:bar":"baz:qux"', }, { description: 'does not split quoted values: case 3', input: 'foo:bar:"baz:qux"', output: ['foo:bar', 'baz:qux'], + warning: 'Should be "foo:bar":"baz:qux"', }, { description: 'does not split quoted values: case 4', @@ -178,13 +202,19 @@ describe('splitFormatDescriptor', () => { description: 'splits string contains multiple ":"', input: 'foo:bar:baz:qux', output: ['foo', 'bar:baz:qux'], + warning: 'Should be "foo":"bar:baz:qux"', }, ] - examples.forEach(({ description, input, output }) => { + examples.forEach(({ description, input, output, warning }) => { it(description, () => { const logger = new FakeLogger() expect(splitFormatDescriptor(logger, input)).to.eql(output) + if (warning) { + expect(logger.warn).to.have.been.calledWith(warning) + } else { + expect(logger.warn).not.to.have.been.called() + } }) }) }) From d6b07b7d228d117049681d70b0dedd42ae6d4a9f Mon Sep 17 00:00:00 2001 From: David Goss Date: Fri, 6 Oct 2023 17:54:00 +0100 Subject: [PATCH 5/8] add deprecation content --- docs/deprecations.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/deprecations.md b/docs/deprecations.md index ab1bac884..15e3b1763 100644 --- a/docs/deprecations.md +++ b/docs/deprecations.md @@ -54,6 +54,17 @@ The `publishQuiet` option (or `--publish-quiet` on the CLI) was used to hide the To adapt, remove the option from your configuration files and CLI commands (especially the latter, since the CLI will fail on unrecognised options). +### Ambiguous colons in formats + +Deprecated in `9.6.0`. Will be removed in `11.0.0` or later. + +User-specified formats where either the formatter name/path or the target path (or both) contains colon(s) are ambiguous because the separator between the two parts is also a colon. Cucumber tries to detect and handle things like Windows drives and `file://` URLs on a best-effort basis, but this logic is being removed in favour of wrapping values in double-quotes. + +| Before | After | +|----------------------------------------------|--------------------------------------------------| +| `html:file://hostname/formatter/report.html` | `"html":"file://hostname/formatter/report.html"` | +| `file://C:\custom\formatter` | `"file://C:\custom\formatter"` | + ## Previous deprecations For deprecations that have been completed (i.e. the functionality removed), see [UPGRADING.md](../UPGRADING.md). From be6dc50b8e26fcf08439f5d3761d04dbfe23c18d Mon Sep 17 00:00:00 2001 From: David Goss Date: Fri, 6 Oct 2023 17:55:09 +0100 Subject: [PATCH 6/8] update the formatters doc --- docs/formatters.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/formatters.md b/docs/formatters.md index be559c77d..d56e840ad 100644 --- a/docs/formatters.md +++ b/docs/formatters.md @@ -7,7 +7,7 @@ cucumber-js provides many built-in Formatters, plus building blocks with which y You can specify one or more formats via the `format` configuration option: - In a configuration file `{ format: ['progress-bar', ['html', 'cucumber-report.html']] }` -- On the CLI `$ cucumber-js --format progress-bar --format html:cucumber-report.html` +- On the CLI `$ cucumber-js --format progress-bar --format "html":"cucumber-report.html"` For each format you specify, you have to provide one or two values. The first (required) is to identify the formatter. It can take a few forms: @@ -18,7 +18,7 @@ For each format you specify, you have to provide one or two values. The first (r Without a second value, the formatter will print to `stdout`. The second value, if present, is a path to where the formatter output should be written. If the path includes directories that do not yet exist, they will be created. -On the CLI, when specifying both a name and path, you'll need to use `:` as a delimiter. In a configuration file you can do this too, but you can also provide an array with the two values as separate strings, which is recommended. +On the CLI, when specifying both a name and path, you'll need to use `:` as a delimiter and wrap each side of it with double quotes. In a configuration file you can do this too, but you can also provide an array with the two values as separate strings, which is recommended. Some notes on specifying Formatters: From 242ffb648392896f620d643c5a1418048360c82a Mon Sep 17 00:00:00 2001 From: David Goss Date: Fri, 6 Oct 2023 18:03:57 +0100 Subject: [PATCH 7/8] make the logging nicer and reference the doc --- src/configuration/split_format_descriptor.ts | 5 ++- .../split_format_descriptor_spec.ts | 41 ++++++++++--------- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/src/configuration/split_format_descriptor.ts b/src/configuration/split_format_descriptor.ts index 10fc805e5..eefdb7efa 100644 --- a/src/configuration/split_format_descriptor.ts +++ b/src/configuration/split_format_descriptor.ts @@ -9,7 +9,10 @@ export function splitFormatDescriptor( if (result[1]) { expected += `:"${result[1]}"` } - logger.warn(`Should be ${expected}`) + logger.warn( + `Each part of a user-specified format should be quoted; see https://github.com/cucumber/cucumber-js/blob/main/docs/deprecations.md#ambiguous-colons-in-formats +Change to ${expected}` + ) } let result: string[] let match1, match2 diff --git a/src/configuration/split_format_descriptor_spec.ts b/src/configuration/split_format_descriptor_spec.ts index 1d956025a..e447ddc80 100644 --- a/src/configuration/split_format_descriptor_spec.ts +++ b/src/configuration/split_format_descriptor_spec.ts @@ -25,7 +25,7 @@ describe('splitFormatDescriptor', () => { input: 'file:///custom/formatter:file:///formatter/output.txt', output: ['file:///custom/formatter', 'file:///formatter/output.txt'], warning: - 'Should be "file:///custom/formatter":"file:///formatter/output.txt"', + 'Change to "file:///custom/formatter":"file:///formatter/output.txt"', }, { description: 'splits file URLs for UNC path', @@ -36,7 +36,7 @@ describe('splitFormatDescriptor', () => { 'file://hostname/formatter/output.txt', ], warning: - 'Should be "file://hostname/custom/formatter":"file://hostname/formatter/output.txt"', + 'Change to "file://hostname/custom/formatter":"file://hostname/formatter/output.txt"', }, { description: 'splits file URLs for absolute windows path', @@ -46,7 +46,7 @@ describe('splitFormatDescriptor', () => { 'file://C:\\formatter\\output.txt', ], warning: - 'Should be "file://C:\\custom\\formatter":"file://C:\\formatter\\output.txt"', + 'Change to "file://C:\\custom\\formatter":"file://C:\\formatter\\output.txt"', }, { description: @@ -57,7 +57,7 @@ describe('splitFormatDescriptor', () => { 'file:///C:/formatter/output.txt', ], warning: - 'Should be "file:///C:/custom/formatter":"file:///C:/formatter/output.txt"', + 'Change to "file:///C:/custom/formatter":"file:///C:/formatter/output.txt"', }, { description: 'splits valid file URLs for absolute windows path', @@ -67,7 +67,7 @@ describe('splitFormatDescriptor', () => { 'file:///C:\\formatter\\output.txt', ], warning: - 'Should be "file:///C:\\custom\\formatter":"file:///C:\\formatter\\output.txt"', + 'Change to "file:///C:\\custom\\formatter":"file:///C:\\formatter\\output.txt"', }, { description: @@ -78,7 +78,7 @@ describe('splitFormatDescriptor', () => { 'file:///C:/formatter/output.txt', ], warning: - 'Should be "file:///C:/custom/formatter":"file:///C:/formatter/output.txt"', + 'Change to "file:///C:/custom/formatter":"file:///C:/formatter/output.txt"', }, { description: 'splits absolute unix paths', @@ -89,14 +89,14 @@ describe('splitFormatDescriptor', () => { description: 'splits absolute windows paths', input: 'C:\\custom\\formatter:C:\\formatter\\output.txt', output: ['C:\\custom\\formatter', 'C:\\formatter\\output.txt'], - warning: 'Should be "C:\\custom\\formatter":"C:\\formatter\\output.txt"', + warning: 'Change to "C:\\custom\\formatter":"C:\\formatter\\output.txt"', }, { description: 'splits absolute windows paths with "/" as directory separator', input: 'C:/custom/formatter:C:/formatter/output.txt', output: ['C:/custom/formatter', 'C:/formatter/output.txt'], - warning: 'Should be "C:/custom/formatter":"C:/formatter/output.txt"', + warning: 'Change to "C:/custom/formatter":"C:/formatter/output.txt"', }, { description: 'splits UNC paths', @@ -125,56 +125,56 @@ describe('splitFormatDescriptor', () => { 'does not split a single file URL for absolute unix path, adds empty string', input: 'file:///custom/formatter', output: ['file:///custom/formatter', ''], - warning: 'Should be "file:///custom/formatter"', + warning: 'Change to "file:///custom/formatter"', }, { description: 'does not split a single file URL for UNC path, adds empty string', input: 'file://hostname/custom/formatter', output: ['file://hostname/custom/formatter', ''], - warning: 'Should be "file://hostname/custom/formatter"', + warning: 'Change to "file://hostname/custom/formatter"', }, { description: 'does not split a single file URL for absolute windows path, adds empty string', input: 'file://C:\\custom\\formatter', output: ['file://C:\\custom\\formatter', ''], - warning: 'Should be "file://C:\\custom\\formatter"', + warning: 'Change to "file://C:\\custom\\formatter"', }, { description: 'does not split a single file URL for absolute windows path with "/" as directory separator, adds empty string', input: 'file://C:/custom/formatter', output: ['file://C:/custom/formatter', ''], - warning: 'Should be "file://C:/custom/formatter"', + warning: 'Change to "file://C:/custom/formatter"', }, { description: 'does not split a valid single file URL for absolute windows path, adds empty string', input: 'file:///C:\\custom\\formatter', output: ['file:///C:\\custom\\formatter', ''], - warning: 'Should be "file:///C:\\custom\\formatter"', + warning: 'Change to "file:///C:\\custom\\formatter"', }, { description: 'does not split a valid single file URL for absolute windows path with "/" as directory separator, adds empty string', input: 'file:///C:/custom/formatter', output: ['file:///C:/custom/formatter', ''], - warning: 'Should be "file:///C:/custom/formatter"', + warning: 'Change to "file:///C:/custom/formatter"', }, { description: 'does not split a single absolute windows path, adds empty string', input: 'C:\\custom\\formatter', output: ['C:\\custom\\formatter', ''], - warning: 'Should be "C:\\custom\\formatter"', + warning: 'Change to "C:\\custom\\formatter"', }, { description: 'does not split a single absolute windows path with "/" as directory separator, adds empty string', input: 'C:/custom/formatter', output: ['C:/custom/formatter', ''], - warning: 'Should be "C:/custom/formatter"', + warning: 'Change to "C:/custom/formatter"', }, { description: 'does not split quoted values: case 1', @@ -185,13 +185,13 @@ describe('splitFormatDescriptor', () => { description: 'does not split quoted values: case 2', input: '"foo:bar":baz:qux', output: ['foo:bar', 'baz:qux'], - warning: 'Should be "foo:bar":"baz:qux"', + warning: 'Change to "foo:bar":"baz:qux"', }, { description: 'does not split quoted values: case 3', input: 'foo:bar:"baz:qux"', output: ['foo:bar', 'baz:qux'], - warning: 'Should be "foo:bar":"baz:qux"', + warning: 'Change to "foo:bar":"baz:qux"', }, { description: 'does not split quoted values: case 4', @@ -202,7 +202,7 @@ describe('splitFormatDescriptor', () => { description: 'splits string contains multiple ":"', input: 'foo:bar:baz:qux', output: ['foo', 'bar:baz:qux'], - warning: 'Should be "foo":"bar:baz:qux"', + warning: 'Change to "foo":"bar:baz:qux"', }, ] @@ -211,7 +211,8 @@ describe('splitFormatDescriptor', () => { const logger = new FakeLogger() expect(splitFormatDescriptor(logger, input)).to.eql(output) if (warning) { - expect(logger.warn).to.have.been.calledWith(warning) + expect(logger.warn).to.have.been.called() + expect(logger.warn.firstCall.firstArg as string).to.contain(warning) } else { expect(logger.warn).not.to.have.been.called() } From c865f5867b26cf2594cac6a588d3324a85dd3241 Mon Sep 17 00:00:00 2001 From: David Goss Date: Fri, 6 Oct 2023 18:18:34 +0100 Subject: [PATCH 8/8] update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 40540d6c1..8e30abeda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ Please see [CONTRIBUTING.md](./CONTRIBUTING.md) on how to contribute to Cucumber ## [Unreleased] ### Fixed - Improve handling of formatter paths ([#2315](https://github.com/cucumber/cucumber-js/pull/2315)) +- Warn on ambiguous colons in formatter paths ([#2335](https://github.com/cucumber/cucumber-js/pull/2335)) ## [9.5.1] - 2023-09-06 ### Fixed