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

Publish option #1424

Merged
merged 45 commits into from
Sep 10, 2020
Merged
Show file tree
Hide file tree
Changes from 38 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
9f1c3d1
Added scenario for --publish option
Sep 1, 2020
b3a2880
Add --publish option
Sep 1, 2020
6ca7dc7
Make publish scenario pass with cheating
Sep 1, 2020
7cbe75d
Add a ReportServer to the World
Sep 1, 2020
56c0888
Start TDDing HttpStream
Sep 1, 2020
1095cf1
Write HttpStream in a temporary file
cbliard Sep 1, 2020
8cdfbca
Pipe tempfile to http request
Sep 1, 2020
5171e70
Wait for request to finish before closing server
Sep 1, 2020
1a00e04
Capture body on server
Sep 1, 2020
227eea9
Wait for the server to receive all the body
cbliard Sep 1, 2020
632eb6a
Add new test for GET/PUT redirect between lambda and S3
Sep 1, 2020
f2fe72d
Follow Location after get
Sep 1, 2020
dd0ebcc
Fix dependency lint
Sep 1, 2020
10f8184
Cleanup
Sep 1, 2020
981a98f
Refactor
Sep 3, 2020
9211f0b
Extract HttpStream
Sep 3, 2020
fc17393
Extract FakeReportServer
Sep 3, 2020
b465540
Use FakeReportServer in publish.feature
Sep 3, 2020
e3f922c
Add failing spec for --publish
Sep 3, 2020
dae139e
Scenario is passing
Sep 3, 2020
c13b8ce
Print response body in errors. Send content-length
Sep 3, 2020
8891803
Added tests for outputting a banner from the report server content
vincent-psarga Sep 4, 2020
f1bdd5e
Merge master. Update Hook signature to allow async functions
Sep 4, 2020
cddacff
http_stream logs response from server to console
vincent-psarga Sep 4, 2020
28ff61b
Enable publishing report with ENV var
vincent-psarga Sep 4, 2020
599c4ae
Fix lint error
Sep 4, 2020
80fb637
Start adding support for CUCUMBER_PUBLISH_TOKEN
Sep 4, 2020
903ff39
Publish with Authorization header when CUCUMBER_PUBLISH_TOKEN is set
vincent-psarga Sep 4, 2020
1b898df
Restore newline
Sep 4, 2020
6276818
Add scenarios describing the banner advertising --publish
vincent-psarga Sep 7, 2020
d7a926f
Start implementing banner display when --publish is not set
vincent-psarga Sep 7, 2020
19ade46
Add isPublishing attribute to configuration
vincent-psarga Sep 7, 2020
231a2d8
Add test checking is suppressPublicationBanner is set + added --publi…
vincent-psarga Sep 7, 2020
be84b19
Merge remote-tracking branch 'origin/master' into publish-option
Sep 7, 2020
4a51358
Print banners to stderr, fixed import of cucumber
Sep 7, 2020
2388754
Adapt banner to cucumber.js
cbliard Sep 7, 2020
5097c2a
Add ANSI colours to banner
Sep 7, 2020
ed63063
Extract banner code to its own file
vincent-psarga Sep 7, 2020
0048be2
more maintainable publish banner
charlierudolph Sep 7, 2020
841d8d6
revert dep lint config, lint fix banner
charlierudolph Sep 7, 2020
508c876
Use arrow function
Sep 8, 2020
9629395
Use doesHaveValue in conditionals. Update comment about 3xx redirects
Sep 8, 2020
860f430
Use valueOrDefault instead of boolean expression
vincent-psarga Sep 10, 2020
272c7bb
Merge branch 'master' into publish-option
vincent-psarga Sep 10, 2020
2bdd55e
Update changelog
vincent-psarga Sep 10, 2020
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
3 changes: 3 additions & 0 deletions cucumber.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const feature = [
'--format rerun:@rerun.txt',
'--format usage:usage.txt',
'--format message:messages.ndjson',
'--publish-quiet',
].join(' ')

const cck = [
Expand All @@ -24,6 +25,7 @@ const FORMATTERS_INCLUDE = [
'parameter-types',
'rules',
'stack-traces',
'--publish-quiet',
]

const formatters = [
Expand All @@ -36,6 +38,7 @@ const formatters = [
`compatibility/features/{${FORMATTERS_INCLUDE.join(',')}}/*.ts`,
'--format',
'message',
'--publish-quiet',
].join(' ')

module.exports = {
Expand Down
3 changes: 2 additions & 1 deletion dependency-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ executedModules:
ignoreErrors:
missing: []
shouldBeDependency: []
shouldBeDevDependency: []
shouldBeDevDependency:
- tmp # used in http_stream.ts as well as in test code
aslakhellesoy marked this conversation as resolved.
Show resolved Hide resolved
unused:
- '@cucumber/compatibility-kit' # files dynamically loaded in cck test, not require'd
- '@typescript-eslint/eslint-plugin' # peer dependency of standard-with-typescript
Expand Down
131 changes: 131 additions & 0 deletions features/publish.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
Feature: Publish reports

Background:
Given a file named "features/a.feature" with:
"""
Feature: a feature
Scenario: a scenario
Given a step
"""
And a file named "features/step_definitions/steps.js" with:
"""
const {Given} = require('@cucumber/cucumber')

Given(/^a step$/, function() {})
"""

@spawn
Scenario: Report is published when --publish is specified
Given a report server is running on 'http://localhost:9987'
When I run cucumber-js with arguments `--publish` and env `CUCUMBER_PUBLISH_URL=http://localhost:9987/api/reports`
Then it passes
And the server should receive the following message types:
| meta |
| source |
| gherkinDocument |
| pickle |
| stepDefinition |
| testRunStarted |
| testCase |
| testCaseStarted |
| testStepStarted |
| testStepFinished |
| testCaseFinished |
| testRunFinished |

@spawn
Scenario: Report is published when CUCUMBER_PUBLISH_ENABLED is set
Given a report server is running on 'http://localhost:9987'
When I run cucumber-js with arguments `` and env `CUCUMBER_PUBLISH_ENABLED=1 CUCUMBER_PUBLISH_URL=http://localhost:9987/api/reports`
Then it passes
And the server should receive the following message types:
| meta |
| source |
| gherkinDocument |
| pickle |
| stepDefinition |
| testRunStarted |
| testCase |
| testCaseStarted |
| testStepStarted |
| testStepFinished |
| testCaseFinished |
| testRunFinished |

@spawn
Scenario: Report is published when CUCUMBER_PUBLISH_TOKEN is set
Given a report server is running on 'http://localhost:9987'
When I run cucumber-js with arguments `` and env `CUCUMBER_PUBLISH_TOKEN=keyboardcat CUCUMBER_PUBLISH_URL=http://localhost:9987/api/reports`
Then it passes
And the server should receive the following message types:
| meta |
| source |
| gherkinDocument |
| pickle |
| stepDefinition |
| testRunStarted |
| testCase |
| testCaseStarted |
| testStepStarted |
| testStepFinished |
| testCaseFinished |
| testRunFinished |
And the server should receive an "Authorization" header with value "Bearer keyboardcat"

@spawn
Scenario: a banner is displayed after publication
Given a report server is running on 'http://localhost:9987'
When I run cucumber-js with arguments `--publish` and env `CUCUMBER_PUBLISH_URL=http://localhost:9987/api/reports`
Then the error output contains the text:
"""
┌──────────────────────────────────────────────────────────────────────────┐
│ View your Cucumber Report at: │
│ https://reports.cucumber.io/reports/f318d9ec-5a3d-4727-adec-bd7b69e2edd3 │
│ │
│ This report will self-destruct in 24h unless it is claimed or deleted. │
└──────────────────────────────────────────────────────────────────────────┘
"""

@spawn
Scenario: when results are not published, a banner explains how to publish
When I run cucumber-js
Then the error output contains the text:
"""
┌──────────────────────────────────────────────────────────────────────────┐
│ Share your Cucumber Report with your team at https://reports.cucumber.io │
│ │
│ Command line option: --publish │
│ Environment variable: CUCUMBER_PUBLISH_ENABLED=true │
│ │
│ More information at https://reports.cucumber.io/docs/cucumber-js │
│ │
│ To disable this message, add this to your ./cucumber.js: │
│ module.exports = { default: '--publish-quiet' } │
Copy link
Member

Choose a reason for hiding this comment

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

I feel conflicted about this. This feels annoying to me as a user. Certainly glad we implemented the CLI option to suppress it but why does this need to be outputted a user runs the tool without publish?

Could this be a message shown on install / the help page / somewhere visible on the README?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We've already implemented it this way in Cucumber-Ruby and Cucumber-JVM. So far we've not heard any complaints, just positive feedback.

We want as many people as possible to discover this new functionality, and showing it after the run will reach more people than if it's only shown on install and in documentation (which many people never look at).

Let's see if anyone complains.

Copy link
Contributor

@michael-lloyd-morris michael-lloyd-morris Oct 22, 2020

Choose a reason for hiding this comment

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

Me. I'll complain. I'm beginning the setup of a test suite and this message is annoying to look at. I tried dropping the module.exports into my step definition file but it didn't get rid of this message. So how do I get rid of this?

And to be clear, I appreciate the thought - but I'm working on a corporate project which will NEVER publish test results onto a public server for security reasons, so I'd rather just have this reminder off.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried dropping the module.exports into my step definition file

You're not supposed to drop that in your step definition files, but in your global cucumber.js config file at the root of your project.

How can we make this harder to misunderstand @michaelm-rsi?

I'm working on a corporate project which will NEVER publish test results onto a public server for security reasons.

I completely understand that, which is why we've added an option to turn this off. We're also considering making Cucumber Reports available as a server you can install on-prem.

└──────────────────────────────────────────────────────────────────────────┘
"""
@spawn
Scenario: the publication banner is not shown when publication is done
When I run cucumber-js with arguments `<args>` and env `<env>`
Then the error output does not contain the text:
"""
Share your Cucumber Report with your team at https://reports.cucumber.io
"""

Examples:
| args | env |
| --publish | |
| | CUCUMBER_PUBLISH_ENABLED=true |
| | CUCUMBER_PUBLISH_TOKEN=123456 |

@spawn
Scenario: the publication banner is not shown when publication is disabled
When I run cucumber-js with arguments `<args>` and env `<env>`
Then the error output does not contain the text:
"""
Share your Cucumber Report with your team at https://reports.cucumber.io
"""

Examples:
| args | env |
| --publish-quiet | |
| | CUCUMBER_PUBLISH_QUIET=true |
30 changes: 30 additions & 0 deletions features/step_definitions/cli_steps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,27 @@ When(
}
)

When(
/^I run cucumber-js with arguments `(|.+)` and env `(|.+)`$/,
{ timeout: 10000 },
async function (this: World, args: string, envs: string) {
const renderedArgs = Mustache.render(valueOrDefault(args, ''), this)
const stringArgs = stringArgv(renderedArgs)
const initialValue: NodeJS.ProcessEnv = {}
const env: NodeJS.ProcessEnv = (envs === null ? '' : envs)
.split(/\s+/)
.map((keyValue) => keyValue.split('='))
.reduce((dict, pair) => {
dict[pair[0]] = pair[1]
return dict
}, initialValue)
return await this.run(this.localExecutablePath, stringArgs, {
...process.env,
...env,
})
}
)

When(
/^I run cucumber-js with all formatters(?: and `(|.+)`)?$/,
{ timeout: 10000 },
Expand Down Expand Up @@ -100,6 +121,15 @@ Then(/^the error output contains the text:$/, function (
expect(actualOutput).to.include(expectedOutput)
})

Then('the error output does not contain the text:', function (
this: World,
text: string
) {
const actualOutput = normalizeText(this.lastRun.errorOutput)
const expectedOutput = normalizeText(text)
expect(actualOutput).not.to.include(expectedOutput)
})

Then(/^I see the version of Cucumber$/, function (this: World) {
const actualOutput = this.lastRun.output
const expectedOutput = `${version as string}\n`
Expand Down
42 changes: 42 additions & 0 deletions features/step_definitions/report_server_steps.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { Given, Then, DataTable } from '../..'
import { World } from '../support/world'
import { expect } from 'chai'
import { URL } from 'url'
import FakeReportServer from '../../test/fake_report_server'
import assert from 'assert'

Given('a report server is running on {string}', async function (
this: World,
url: string
) {
const port = parseInt(new URL(url).port)
this.reportServer = new FakeReportServer(port)
await this.reportServer.start()
})

Then('the server should receive the following message types:', async function (
this: World,
expectedMessageTypesTable: DataTable
) {
const expectedMessageTypes = expectedMessageTypesTable
.raw()
.map((row) => row[0])

const receivedBodies = await this.reportServer.stop()
const ndjson = receivedBodies.toString('utf-8').trim()
if (ndjson === '') assert.fail('Server received nothing')

const receivedMessageTypes = ndjson
.split(/\n/)
.map((line) => JSON.parse(line))
.map((envelope) => Object.keys(envelope)[0])

expect(receivedMessageTypes).to.deep.eq(expectedMessageTypes)
})

Then(
'the server should receive a(n) {string} header with value {string}',
function (this: World, name: string, value: string) {
expect(this.reportServer.receivedHeaders[name.toLowerCase()]).to.eq(value)
}
)
6 changes: 6 additions & 0 deletions features/support/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,3 +106,9 @@ After(function (this: World) {
)
}
})

After(async function (this: World) {
if (this.reportServer?.started) {
await this.reportServer.stop()
}
})
19 changes: 15 additions & 4 deletions features/support/world.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import VError from 'verror'
import _ from 'lodash'
import ndjsonParse from 'ndjson-parse'
import { messages } from '@cucumber/messages'
import FakeReportServer from '../../test/fake_report_server'

interface ILastRun {
error: any
Expand All @@ -32,8 +33,13 @@ export class World {
public verifiedLastRunError: boolean
public localExecutablePath: string
public globalExecutablePath: string
public reportServer: FakeReportServer

async run(executablePath: string, inputArgs: string[]): Promise<void> {
async run(
executablePath: string,
inputArgs: string[],
env: NodeJS.ProcessEnv = process.env
): Promise<void> {
const messageFilename = 'message.ndjson'
const args = ['node', executablePath]
.concat(inputArgs, [
Expand All @@ -54,9 +60,14 @@ export class World {

if (this.spawn) {
result = await new Promise((resolve) => {
execFile(args[0], args.slice(1), { cwd }, (error, stdout, stderr) => {
resolve({ error, stdout, stderr })
})
execFile(
args[0],
args.slice(1),
{ cwd, env },
(error, stdout, stderr) => {
resolve({ error, stdout, stderr })
}
)
})
} else {
const stdout = new PassThrough()
Expand Down
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@
"stack-chain": "^2.0.0",
"stacktrace-js": "^2.0.2",
"string-argv": "^0.3.1",
"tmp": "^0.2.1",
"util-arity": "^1.1.0",
"verror": "^1.10.0"
},
Expand All @@ -193,6 +194,7 @@
"@types/bluebird": "3.5.32",
"@types/chai": "4.2.12",
"@types/dirty-chai": "2.0.2",
"@types/express": "^4.17.7",
"@types/fs-extra": "9.0.1",
"@types/glob": "7.1.3",
"@types/lodash": "4.14.161",
Expand Down Expand Up @@ -228,6 +230,7 @@
"eslint-plugin-prettier": "3.1.4",
"eslint-plugin-promise": "4.2.1",
"eslint-plugin-standard": "4.0.1",
"express": "^4.17.1",
"fs-extra": "9.0.1",
"mocha": "8.1.3",
"mustache": "4.0.1",
Expand All @@ -241,7 +244,6 @@
"sinon-chai": "3.5.0",
"stream-buffers": "3.0.2",
"stream-to-string": "1.2.0",
"tmp": "0.2.1",
"ts-node": "9.0.0",
"tsify": "5.0.2",
"typescript": "4.0.2"
Expand Down
12 changes: 12 additions & 0 deletions src/cli/argv_parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ export interface IParsedArgvOptions {
parallel: number
predictableIds: boolean
profile: string[]
publish: boolean
publishQuiet: boolean
require: string[]
requireModule: string[]
retry: number
Expand Down Expand Up @@ -168,6 +170,16 @@ const ArgvParser = {
'Use predictable ids in messages (option ignored if using parallel)',
false
)
.option(
'--publish',
'Publish a report to https://reports.cucumber.io',
false
)
.option(
'--publish-quiet',
vincent-psarga marked this conversation as resolved.
Show resolved Hide resolved
"Don't print information banner about publishing reports",
false
)
.option(
'-r, --require <GLOB|DIR|FILE>',
'require files before executing features (repeatable)',
Expand Down
Loading