Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

formatters: include html-formatter as a built-in option #1432

Merged
merged 19 commits into from
Oct 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ Please see [CONTRIBUTING.md](https://github.com/cucumber/cucumber/blob/master/CO

### Added

* Add a built in `html` formatter for rich HTML reports output as a standalone page

### Changed

### Deprecated
Expand Down
6 changes: 3 additions & 3 deletions cucumber.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const FORMATTERS_INCLUDE = [
'--publish-quiet',
]

const formatters = [
const htmlFormatter = [
`node_modules/@cucumber/compatibility-kit/features/{${FORMATTERS_INCLUDE.join(
','
)}}/*.feature`,
Expand All @@ -37,12 +37,12 @@ const formatters = [
'--require',
`compatibility/features/{${FORMATTERS_INCLUDE.join(',')}}/*.ts`,
'--format',
'message',
'html:html-formatter.html',
'--publish-quiet',
].join(' ')

module.exports = {
default: feature,
cck,
formatters,
htmlFormatter,
}
1 change: 1 addition & 0 deletions docs/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ If multiple formats are specified with the same output, only the last is used.

Built-in formatters
* message - prints each [message](https://github.com/cucumber/cucumber/tree/master/cucumber-messages) in NDJSON form, which can then be consumed by other tools.
* html - prints a rich HTML report to a standalone page
* json - prints the feature as JSON. *Note: this formatter is deprecated and will be removed in the next major release. Where you need a structured data representation of your test run, it's best to use the `message` formatter. For legacy tools that depend on the deprecated JSON format, a standalone formatter is available (see https://github.com/cucumber/cucumber/tree/master/json-formatter).
* progress - prints one character per scenario (default).
* progress-bar - prints a progress bar and outputs errors/warnings along the way.
Expand Down
5 changes: 5 additions & 0 deletions features/formatters.feature
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Feature: Formatters
When I run cucumber-js with all formatters and `--tags @a`
Then the "message" formatter output matches the fixture "formatters/rejected-pickle.message.json"
Then the "json" formatter output matches the fixture "formatters/rejected-pickle.json"
Then the html formatter output is complete

Scenario: passed from Scenario
Given a file named "features/a.feature" with:
Expand All @@ -27,6 +28,7 @@ Feature: Formatters
When I run cucumber-js with all formatters
Then the "message" formatter output matches the fixture "formatters/passed-scenario.message.json"
Then the "json" formatter output matches the fixture "formatters/passed-scenario.json"
Then the html formatter output is complete

Scenario: passed from Rule
Given a file named "features/a.feature" with:
Expand All @@ -45,6 +47,7 @@ Feature: Formatters
When I run cucumber-js with all formatters
Then the "message" formatter output matches the fixture "formatters/passed-rule.message.json"
Then the "json" formatter output matches the fixture "formatters/passed-rule.json"
Then the html formatter output is complete

Scenario: failed
Given a file named "features/a.feature" with:
Expand All @@ -62,6 +65,7 @@ Feature: Formatters
When I run cucumber-js with all formatters
Then the "message" formatter output matches the fixture "formatters/failed.message.json"
Then the "json" formatter output matches the fixture "formatters/failed.json"
Then the html formatter output is complete
And it fails

Scenario: retried and passed
Expand Down Expand Up @@ -89,3 +93,4 @@ Feature: Formatters
When I run cucumber-js with all formatters and `--retry 1`
Then the "message" formatter output matches the fixture "formatters/retried.message.json"
Then the "json" formatter output matches the fixture "formatters/retried.json"
Then the html formatter output is complete
2 changes: 1 addition & 1 deletion features/step_definitions/cli_steps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ When(
args = ''
}
// message is always outputted as part of run
const formats = ['json:json.out']
const formats = ['html:html.out', 'json:json.out']
args += ' ' + formats.map((f) => `--format ${f}`).join(' ')
const renderedArgs = Mustache.render(args, this)
const stringArgs = stringArgv(renderedArgs)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,10 @@ Then(
expect(actual).to.eql(expected)
}
)

Then('the html formatter output is complete', async function (this: World) {
const actualPath = path.join(this.tmpDir, `html.out`)
const actual = await fs.readFile(actualPath, 'utf8')
expect(actual).to.contain('<html lang="en">')
expect(actual).to.contain('</html>')
davidjgoss marked this conversation as resolved.
Show resolved Hide resolved
})
8 changes: 5 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@
"@cucumber/create-meta": "^2.0.2",
"@cucumber/cucumber-expressions": "^10.3.0",
"@cucumber/gherkin": "^15.0.2",
"@cucumber/html-formatter": "^9.0.0",
"@cucumber/messages": "^13.0.1",
"@cucumber/query": "^7.0.0",
"@cucumber/tag-expressions": "^2.0.4",
Expand All @@ -188,6 +189,7 @@
"mz": "^2.7.0",
"progress": "^2.0.3",
"resolve": "^1.17.0",
"resolve-pkg": "^2.0.0",
"stack-chain": "^2.0.0",
"stacktrace-js": "^2.0.2",
"string-argv": "^0.3.1",
Expand Down Expand Up @@ -261,7 +263,7 @@
"build-release": "browserify scripts/cucumber.ts -o dist/cucumber.js -p [ tsify -p tsconfig.browser.json ] --standalone Cucumber --debug",
"cck-test": "mocha 'compatibility/**/*_spec.ts'",
"feature-test": "node ./bin/cucumber-js",
"formatters-test": "node ./bin/cucumber-js --profile formatters | npx @cucumber/html-formatter@9.0.0 --format ndjson > html-formatter.html",
"html-formatter": "node ./bin/cucumber-js --profile htmlFormatter",
"lint-autofix": "eslint --fix \"{compatibility,example,features,scripts,src,test}/**/*.ts\"",
"lint-code": "eslint \"{compatibility,example,features,scripts,src,test}/**/*.ts\"",
"lint-dependencies": "dependency-lint",
Expand All @@ -272,8 +274,8 @@
"prefeature-test": "yarn run build-local",
"prepublishOnly": "rm -rf lib && npm run build-local",
"test-browser-example": "yarn build-release && yarn build-browser-example && git checkout -- dist/cucumber.js",
"test-coverage": "yarn run lint && ./scripts/test-coverage.sh && yarn run formatters-test",
"test": "yarn run lint && yarn run unit-test && yarn run cck-test && yarn run feature-test && yarn run formatters-test",
"test-coverage": "yarn run lint && ./scripts/test-coverage.sh",
"test": "yarn run lint && yarn run unit-test && yarn run cck-test && yarn run feature-test",
"unit-test": "mocha 'src/**/*_spec.ts'",
"update-dependencies": "npx npm-check-updates --upgrade"
},
Expand Down
83 changes: 44 additions & 39 deletions src/cli/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,50 +88,55 @@ export default class Cli {
formats,
supportCodeLibrary,
}: IInitializeFormattersRequest): Promise<() => Promise<void>> {
const streamsToClose: IFormatterStream[] = []
await bluebird.map(formats, async ({ type, outputTo }) => {
let stream: IFormatterStream = this.stdout
if (outputTo !== '') {
if (outputTo.match(new RegExp('^https?://')) !== null) {
const headers: { [key: string]: string } = {}
if (process.env.CUCUMBER_PUBLISH_TOKEN !== undefined) {
headers.Authorization = `Bearer ${process.env.CUCUMBER_PUBLISH_TOKEN}`
}
const formatters = await bluebird.map(
formats,
async ({ type, outputTo }) => {
let stream: IFormatterStream = this.stdout
if (outputTo !== '') {
if (outputTo.match(new RegExp('^https?://')) !== null) {
const headers: { [key: string]: string } = {}
if (process.env.CUCUMBER_PUBLISH_TOKEN !== undefined) {
headers.Authorization = `Bearer ${process.env.CUCUMBER_PUBLISH_TOKEN}`
}

stream = new HttpStream(outputTo, 'GET', headers, (content) =>
console.error(content)
stream = new HttpStream(outputTo, 'GET', headers, (content) =>
console.error(content)
)
} else {
const fd = await fs.open(path.resolve(this.cwd, outputTo), 'w')
stream = fs.createWriteStream(null, { fd })
}
}
const typeOptions = {
cwd: this.cwd,
eventBroadcaster,
eventDataCollector,
log: stream.write.bind(stream),
parsedArgvOptions: formatOptions,
stream,
cleanup:
stream === this.stdout
? async () => await Promise.resolve()
: bluebird.promisify(stream.end.bind(stream)),
supportCodeLibrary,
}
if (doesNotHaveValue(formatOptions.colorsEnabled)) {
typeOptions.parsedArgvOptions.colorsEnabled = (stream as TtyWriteStream).isTTY
}
if (type === 'progress-bar' && !(stream as TtyWriteStream).isTTY) {
const outputToName = outputTo === '' ? 'stdout' : outputTo
console.warn(
`Cannot use 'progress-bar' formatter for output to '${outputToName}' as not a TTY. Switching to 'progress' formatter.`
)
} else {
const fd = await fs.open(path.resolve(this.cwd, outputTo), 'w')
stream = fs.createWriteStream(null, { fd })
type = 'progress'
}
streamsToClose.push(stream)
return FormatterBuilder.build(type, typeOptions)
}
const typeOptions = {
cwd: this.cwd,
eventBroadcaster,
eventDataCollector,
log: stream.write.bind(stream),
parsedArgvOptions: formatOptions,
stream,
supportCodeLibrary,
}
if (doesNotHaveValue(formatOptions.colorsEnabled)) {
typeOptions.parsedArgvOptions.colorsEnabled = (stream as TtyWriteStream).isTTY
}
if (type === 'progress-bar' && !(stream as TtyWriteStream).isTTY) {
const outputToName = outputTo === '' ? 'stdout' : outputTo
console.warn(
`Cannot use 'progress-bar' formatter for output to '${outputToName}' as not a TTY. Switching to 'progress' formatter.`
)
type = 'progress'
}
return FormatterBuilder.build(type, typeOptions)
})
)
return async function () {
await bluebird.each(streamsToClose, (stream) =>
bluebird.promisify(stream.end.bind(stream))()
)
await bluebird.each(formatters, async (formatter) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost all of the rest of this file is just indentation changes - this is the addition, where we wait for each formatter to be finished.

Copy link
Member

@charlierudolph charlierudolph Sep 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on making the default formatter finished function end the stream?
Then for the html formatter, it waits for the cucumber html stream to finish and then also awaits on super? Think that simplifies this a little bit.

Copy link
Contributor Author

@davidjgoss davidjgoss Sep 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That does seem like a good balance.

In cases where the formatter has asked to write to stdout, we can't just end the stream when the formatter is done writing, because the process still needs stdout. The current code accounts for this by not adding it to the list of streams to end.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a formatter's writable stream is written to by piping another readable stream into it, then we just have to wait until the pipe is finished. We cannot prematurely close it.

This is what's happening in this case. The CucumberHtmlStream is piped to the formatter's stream. We have no control over when it's done.

Copy link
Contributor Author

@davidjgoss davidjgoss Sep 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I have pushed something which I believe accounts for all of this whilst keeping a straightforward API for formatter authors - though not sure it really comes out any simpler. Let me know what you think. Also clarified my comment above.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That does seem like a good balance.

In cases where the formatter has asked to write to stdout, we can't just end the stream when the formatter is done writing, because the process still needs stdout. The current code accounts for this by not adding it to the list of streams to end.

I think it would be best to wait for it to have written everything. The correct way to keep stdout open is to stream.pipe(process.stdout, {end: false})

await formatter.finished()
})
}
}

Expand Down
6 changes: 5 additions & 1 deletion src/formatter/builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,14 @@ import SummaryFormatter from './summary_formatter'
import UsageFormatter from './usage_formatter'
import UsageJsonFormatter from './usage_json_formatter'
import { ISupportCodeLibrary } from '../support_code_library_builder/types'
import Formatter, { IFormatterLogFn } from '.'
import Formatter, { IFormatterCleanupFn, IFormatterLogFn } from '.'
import { doesHaveValue, doesNotHaveValue } from '../value_checker'
import { EventEmitter } from 'events'
import EventDataCollector from './helpers/event_data_collector'
import { Writable as WritableStream } from 'stream'
import { IParsedArgvFormatOptions } from '../cli/argv_parser'
import { SnippetInterface } from './step_definition_snippet_builder/snippet_syntax'
import HtmlFormatter from './html_formatter'

interface IGetStepDefinitionSnippetBuilderOptions {
cwd: string
Expand All @@ -34,6 +35,7 @@ export interface IBuildOptions {
log: IFormatterLogFn
parsedArgvOptions: IParsedArgvFormatOptions
stream: WritableStream
cleanup: IFormatterCleanupFn
supportCodeLibrary: ISupportCodeLibrary
}

Expand Down Expand Up @@ -63,6 +65,8 @@ const FormatterBuilder = {
return JsonFormatter
case 'message':
return MessageFormatter
case 'html':
return HtmlFormatter
case 'progress':
return ProgressFormatter
case 'progress-bar':
Expand Down
34 changes: 34 additions & 0 deletions src/formatter/html_formatter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import Formatter, { IFormatterOptions } from '.'
import { messages } from '@cucumber/messages'
import resolvePkg from 'resolve-pkg'
import CucumberHtmlStream from '@cucumber/html-formatter'
import { doesHaveValue } from '../value_checker'
import { finished } from 'stream'
import { promisify } from 'util'

export default class HtmlFormatter extends Formatter {
private readonly _finished: Promise<void>

constructor(options: IFormatterOptions) {
super(options)
const cucumberHtmlStream = new CucumberHtmlStream(
resolvePkg('@cucumber/react', { cwd: __dirname }) +
'/dist/src/styles/cucumber-react.css',
resolvePkg('@cucumber/html-formatter', { cwd: __dirname }) +
'/dist/main.js'
)
options.eventBroadcaster.on('envelope', (envelope: messages.Envelope) => {
cucumberHtmlStream.write(envelope)
if (doesHaveValue(envelope.testRunFinished)) {
cucumberHtmlStream.end()
}
})
cucumberHtmlStream.on('data', (chunk) => this.log(chunk))
this._finished = promisify(finished)(cucumberHtmlStream)
}

async finished(): Promise<void> {
await this._finished
await super.finished()
}
}
8 changes: 8 additions & 0 deletions src/formatter/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export type IFormatterStream =
| PassThrough
| HttpStream
export type IFormatterLogFn = (buffer: string | Uint8Array) => void
export type IFormatterCleanupFn = () => Promise<any>

export interface IFormatterOptions {
colorFns: IColorFns
Expand All @@ -25,6 +26,7 @@ export interface IFormatterOptions {
parsedArgvOptions: IParsedArgvFormatOptions
snippetBuilder: StepDefinitionSnippetBuilder
stream: WritableStream
cleanup: IFormatterCleanupFn
supportCodeLibrary: ISupportCodeLibrary
}

Expand All @@ -36,6 +38,7 @@ export default class Formatter {
protected snippetBuilder: StepDefinitionSnippetBuilder
protected stream: WritableStream
protected supportCodeLibrary: ISupportCodeLibrary
private readonly cleanup: IFormatterCleanupFn

constructor(options: IFormatterOptions) {
this.colorFns = options.colorFns
Expand All @@ -45,5 +48,10 @@ export default class Formatter {
this.snippetBuilder = options.snippetBuilder
this.stream = options.stream
this.supportCodeLibrary = options.supportCodeLibrary
this.cleanup = options.cleanup
}

async finished(): Promise<void> {
await this.cleanup()
}
}
5 changes: 4 additions & 1 deletion src/formatter/progress_bar_formatter_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import ProgressBarFormatter from './progress_bar_formatter'
import { doesHaveValue, doesNotHaveValue } from '../value_checker'
import { PassThrough } from 'stream'
import ProgressBar from 'progress'
import bluebird from 'bluebird'

interface ITestProgressBarFormatterOptions {
runtimeOptions?: Partial<IRuntimeOptions>
Expand Down Expand Up @@ -53,13 +54,15 @@ async function testProgressBarFormatter({
const logFn = (data: string): void => {
output += data
}
const passThrough = new PassThrough()
const progressBarFormatter = FormatterBuilder.build('progress-bar', {
cwd: '',
eventBroadcaster,
eventDataCollector: new EventDataCollector(eventBroadcaster),
log: logFn,
parsedArgvOptions: {},
stream: new PassThrough(),
stream: passThrough,
cleanup: bluebird.promisify(passThrough.end.bind(passThrough)),
supportCodeLibrary,
}) as ProgressBarFormatter
let mocked = false
Expand Down
5 changes: 4 additions & 1 deletion test/formatter_helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { doesNotHaveValue } from '../src/value_checker'
import { IParsedArgvFormatOptions } from '../src/cli/argv_parser'
import { PassThrough } from 'stream'
import { emitSupportCodeMessages } from '../src/cli/helpers'
import bluebird from 'bluebird'
import IEnvelope = messages.IEnvelope

const { uuid } = IdGenerator
Expand Down Expand Up @@ -57,13 +58,15 @@ export async function testFormatter({
const logFn = (data: string): void => {
output += data
}
const passThrough = new PassThrough()
FormatterBuilder.build(type, {
cwd: '',
eventBroadcaster,
eventDataCollector,
log: logFn,
parsedArgvOptions,
stream: new PassThrough(),
stream: passThrough,
cleanup: bluebird.promisify(passThrough.end.bind(passThrough)),
supportCodeLibrary,
})
let pickleIds: string[] = []
Expand Down
Loading