Skip to content

Commit

Permalink
fix: ensure absolute paths (as file urls) can be loaded for formatter…
Browse files Browse the repository at this point in the history
…s, snippets (#1963)

* have option splitter handle file urls

* test for handling file url when loading class

* add changelog entry

* handle property in convertConfiguration

* update doc

* update CHANGELOG.md

* add retroactive changelog entry for rc1
  • Loading branch information
davidjgoss committed Mar 25, 2022
1 parent aa3972a commit 0acdfe0
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 30 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -8,6 +8,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
Please see [CONTRIBUTING.md](https://github.com/cucumber/cucumber/blob/master/CONTRIBUTING.md) on how to contribute to Cucumber.

## [Unreleased]
### Fixed
- Allow `file://` URLs to be used as formatter/snippet paths in options ([#1963](https://github.com/cucumber/cucumber-js/pull/1963) [#1920](https://github.com/cucumber/cucumber-js/issues/1920))

## [8.0.0-rc.3] - 2022-03-21
### Added
Expand Down Expand Up @@ -66,6 +68,7 @@ See [docs/profiles.md](./docs/profiles.md#using-another-file-than-cucumberjs) fo

### Changed
- Relative paths for custom snippet syntaxes must begin with `.` ([#1640](https://github.com/cucumber/cucumber-js/issues/1640))
- Absolute paths for custom formatters and snippet syntaxes must be a valid `file://` URL
- Use performance timers for test case duration measurement.
[#1793](https://github.com/cucumber/cucumber-js/pull/1793)

Expand Down
1 change: 1 addition & 0 deletions docs/formatters.md
Expand Up @@ -14,6 +14,7 @@ For each value you provide, `TYPE` should be one of:
* The name of one of the built-in formatters (below) e.g. `progress`
* A module/package name e.g. `@cucumber/pretty-formatter`
* A relative path to a local formatter implementation e.g. `./my-customer-formatter.js`
* An absolute path to a local formatter implementation in the form of a `file://` URL

If `PATH` is supplied, the formatter prints to the given file, otherwise it prints to `stdout`.

Expand Down
54 changes: 31 additions & 23 deletions src/api/convert_configuration.ts
Expand Up @@ -32,35 +32,32 @@ export async function convertConfiguration(
strict: flatConfiguration.strict,
worldParameters: flatConfiguration.worldParameters,
},
formats: {
stdout:
[...flatConfiguration.format]
.reverse()
.find((option) => !option.includes(':')) ?? 'progress',
files: flatConfiguration.format
.filter((option) => option.includes(':'))
.reduce((mapped, item) => {
const [type, target] = OptionSplitter.split(item)
return {
...mapped,
[target]: type,
}
}, {}),
publish: makePublishConfig(flatConfiguration, env),
options: flatConfiguration.formatOptions,
},
formats: convertFormats(flatConfiguration, env),
}
}

function isPublishing(
function convertFormats(
flatConfiguration: IConfiguration,
env: NodeJS.ProcessEnv
): boolean {
return (
flatConfiguration.publish ||
isTruthyString(env.CUCUMBER_PUBLISH_ENABLED) ||
env.CUCUMBER_PUBLISH_TOKEN !== undefined
) {
const splitFormats: string[][] = flatConfiguration.format.map((item) =>
OptionSplitter.split(item)
)
return {
stdout:
[...splitFormats].reverse().find(([, target]) => !target)?.[0] ??
'progress',
files: splitFormats
.filter(([, target]) => !!target)
.reduce((mapped, [type, target]) => {
return {
...mapped,
[target]: type,
}
}, {}),
publish: makePublishConfig(flatConfiguration, env),
options: flatConfiguration.formatOptions,
}
}

function makePublishConfig(
Expand All @@ -76,3 +73,14 @@ function makePublishConfig(
token: env.CUCUMBER_PUBLISH_TOKEN,
}
}

function isPublishing(
flatConfiguration: IConfiguration,
env: NodeJS.ProcessEnv
): boolean {
return (
flatConfiguration.publish ||
isTruthyString(env.CUCUMBER_PUBLISH_ENABLED) ||
env.CUCUMBER_PUBLISH_TOKEN !== undefined
)
}
24 changes: 24 additions & 0 deletions src/api/convert_configuration_spec.ts
Expand Up @@ -63,4 +63,28 @@ describe('convertConfiguration', () => {
options: {},
})
})

it('should map formatters correctly when file:// urls are involved', async () => {
const result = await convertConfiguration(
{
...DEFAULT_CONFIGURATION,
format: [
'file:///my/fancy/formatter',
'json:./report.json',
'html:./report.html',
],
},
{}
)

expect(result.formats).to.eql({
stdout: 'file:///my/fancy/formatter',
files: {
'./report.html': 'html',
'./report.json': 'json',
},
publish: false,
options: {},
})
})
})
2 changes: 1 addition & 1 deletion src/configuration/option_splitter.ts
Expand Up @@ -2,7 +2,7 @@ export const OptionSplitter = {
split(option: string): string[] {
option = option.replace(/"/g, '')

const parts = option.split(/([^A-Z]):(?!\\)/)
const parts = option.split(/((?:file){0}):(?!\\|\/\/)/)

const result = parts.reduce((memo: string[], part: string, i: number) => {
if (partNeedsRecombined(i)) {
Expand Down
15 changes: 9 additions & 6 deletions src/configuration/option_splitter_spec.ts
Expand Up @@ -16,8 +16,8 @@ describe('OptionSplitter', () => {
},
{
description: 'splits absolute unix paths',
input: '/custom/formatter:/formatter/output.txt',
output: ['/custom/formatter', '/formatter/output.txt'],
input: 'file:///custom/formatter:file:///formatter/output.txt',
output: ['file:///custom/formatter', 'file:///formatter/output.txt'],
},
{
description: 'splits paths with quotes around them',
Expand All @@ -26,14 +26,17 @@ describe('OptionSplitter', () => {
},
{
description: 'splits absolute windows paths',
input: 'C:\\custom\\formatter:C:\\formatter\\output.txt',
output: ['C:\\custom\\formatter', 'C:\\formatter\\output.txt'],
input: 'file://C:\\custom\\formatter:file://C:\\formatter\\output.txt',
output: [
'file://C:\\custom\\formatter',
'file://C:\\formatter\\output.txt',
],
},
{
description:
'does not split a single absolute windows paths, adds empty string',
input: 'C:\\custom\\formatter',
output: ['C:\\custom\\formatter', ''],
input: 'file://C:\\custom\\formatter',
output: ['file://C:\\custom\\formatter', ''],
},
]

Expand Down
15 changes: 15 additions & 0 deletions src/formatter/builder_spec.ts
@@ -1,7 +1,22 @@
import { expect } from 'chai'
import FormatterBuilder from './builder'
import { pathToFileURL } from 'url'
import path from 'path'

describe('custom class loading', () => {
it('should handle a file:// url', async () => {
const fileUrl = pathToFileURL(
path.resolve(__dirname, './fixtures/module_dot_exports.cjs')
).toString()
const CustomClass = await FormatterBuilder.loadCustomClass(
'formatter',
fileUrl,
__dirname
)

expect(typeof CustomClass).to.eq('function')
})

it('should handle cjs module.exports', async () => {
const CustomClass = await FormatterBuilder.loadCustomClass(
'formatter',
Expand Down

0 comments on commit 0acdfe0

Please sign in to comment.