support: record step keyword, deprecate defineStep (#2044)

* mark defineStep as deprecated on entry point

* failing test and a bit of plumbing

* scrappy implementation

* split out a type for the step function

* better implementation

* rename type

* update documentation

* add changelog entry

* update CHANGELOG.md

* rename interface for consistency

* add runtime deprecation warning

* use Unknown keyword for defineStep

CHANGELOG.md

@@ -11,6 +11,9 @@ Please see [CONTRIBUTING.md](https://github.com/cucumber/cucumber/blob/master/CO
 ### Added
 - Add `willBeRetried` to the parameter passed to `After` hook functions ([#2045](https://github.com/cucumber/cucumber-js/pull/2045))
 
+### Changed
+- `defineStep` is now deprecated and will eventually be removed; use the appropriate Given/When/Then keyword to define your step ([#2044](https://github.com/cucumber/cucumber-js/pull/2044))
+
 ### Fixed
 - Prevent outputting ANSI escapes to `stderr` if it can't display them ([#2035](https://github.com/cucumber/cucumber-js/pull/2035))
 

docs/support_files/api_reference.md

@@ -104,11 +104,11 @@ Multiple `BeforeStep` hooks are executed in the order that they are defined.
 
 ---
 
-#### `defineStep(pattern[, options], fn)`
+#### `Given(pattern[, options], fn)`
 
-Defines a step.
+Define a "Given" step.
 
-Aliases: `Given`, `When`, `Then`.
+Aliases: `defineStep` (deprecated and will be removed in a future release; use the appropriate Given/When/Then keyword to define your step).
 
 * `pattern`: A regex or string pattern to match against a gherkin step.
 * `options`: An object with the following keys:
@@ -121,12 +121,6 @@ Aliases: `Given`, `When`, `Then`.
 
 ---
 
-#### `Given(pattern[, options], fn)`
-
-Alias of `defineStep`.
-
----
-
 #### `setDefaultTimeout(milliseconds)`
 
 Set the default timeout for asynchronous steps. Defaults to `5000` milliseconds.
@@ -199,10 +193,10 @@ function World({attach, parameters}) {
 
 #### `Then(pattern[, options], fn)`
 
-Alias of `defineStep`.
+Define a "Then" step. Same interface as `Given`
 
 ---
 
 #### `When(pattern[, options], fn)`
 
-Alias of `defineStep`.
+Define a "When" step. Same interface as `Given`

src/cli/helpers_spec.ts

@@ -145,6 +145,7 @@ describe('helpers', () => {
             line: 9,
             options: {},
             uri: 'features/support/cukes.js',
+            keyword: 'Given',
             pattern: 'I have {int} cukes in my belly',
             expression: new CucumberExpression(
               'I have {int} cukes in my belly',
@@ -183,6 +184,7 @@ describe('helpers', () => {
             line: 9,
             options: {},
             uri: 'features/support/cukes.js',
+            keyword: 'Given',
             pattern: /I have (\d+) cukes in my belly/,
             expression: new RegularExpression(
               /I have (\d+) cukes in my belly/,

src/index.ts

@@ -38,6 +38,9 @@ export const Before = methods.Before
 export const BeforeAll = methods.BeforeAll
 export const BeforeStep = methods.BeforeStep
 export const defineParameterType = methods.defineParameterType
+/**
+ * @deprecated use `Given`, `When` or `Then` instead; see <https://github.com/cucumber/cucumber-js/issues/2043>
+ */
 export const defineStep = methods.defineStep
 export const Given = methods.Given
 export const setDefaultTimeout = methods.setDefaultTimeout

src/models/definition.ts

@@ -1,6 +1,7 @@
 import * as messages from '@cucumber/messages'
 import { ITestCaseHookParameter } from '../support_code_library_builder/types'
 import { Expression } from '@cucumber/cucumber-expressions'
+import { GherkinStepKeyword } from './gherkin_step_keyword'
 
 export interface IGetInvocationDataRequest {
   hookParameter: ITestCaseHookParameter
@@ -35,6 +36,7 @@ export interface IDefinitionParameters<T extends IDefinitionOptions> {
 
 export interface IStepDefinitionParameters
   extends IDefinitionParameters<IDefinitionOptions> {
+  keyword: GherkinStepKeyword
   pattern: string | RegExp
   expression: Expression
 }

src/models/gherkin_step_keyword.ts

@@ -0,0 +1 @@
+export type GherkinStepKeyword = 'Unknown' | 'Given' | 'When' | 'Then'

src/models/step_definition.ts

@@ -8,13 +8,16 @@ import Definition, {
 import { parseStepArgument } from '../step_arguments'
 import { Expression } from '@cucumber/cucumber-expressions'
 import { doesHaveValue } from '../value_checker'
+import { GherkinStepKeyword } from './gherkin_step_keyword'
 
 export default class StepDefinition extends Definition implements IDefinition {
+  public readonly keyword: GherkinStepKeyword
   public readonly pattern: string | RegExp
   public readonly expression: Expression
 
   constructor(data: IStepDefinitionParameters) {
     super(data)
+    this.keyword = data.keyword
     this.pattern = data.pattern
     this.expression = data.expression
   }

src/runtime/helpers_spec.ts

@@ -16,6 +16,7 @@ describe('Helpers', () => {
           id: '',
           options: undefined,
           line: 3,
+          keyword: 'Given',
           pattern: 'pattern1',
           uri: 'steps1.js',
         }),
@@ -25,6 +26,7 @@ describe('Helpers', () => {
           id: '',
           options: undefined,
           line: 4,
+          keyword: 'Given',
           pattern: 'longer pattern2',
           uri: 'steps2.js',
         }),

src/support_code_library_builder/index.ts

@@ -8,6 +8,7 @@ import TestRunHookDefinition from '../models/test_run_hook_definition'
 import StepDefinition from '../models/step_definition'
 import { formatLocation } from '../formatter/helpers'
 import validateArguments from './validate_arguments'
+import { deprecate } from 'util'
 import arity from 'util-arity'
 
 import {
@@ -29,14 +30,17 @@ import {
   TestStepHookFunction,
   ParallelAssignmentValidator,
   ISupportCodeCoordinates,
+  IDefineStep,
 } from './types'
 import World from './world'
 import { ICanonicalSupportCodeIds } from '../runtime/parallel/command_types'
+import { GherkinStepKeyword } from '../models/gherkin_step_keyword'
 
 interface IStepDefinitionConfig {
   code: any
   line: number
   options: any
+  keyword: GherkinStepKeyword
   pattern: string | RegExp
   uri: string
 }
@@ -96,7 +100,6 @@ export class SupportCodeLibraryBuilder {
   private parallelCanAssign: ParallelAssignmentValidator
 
   constructor() {
-    const defineStep = this.defineStep.bind(this)
     this.methods = {
       After: this.defineTestCaseHook(
         () => this.afterTestCaseHookDefinitionConfigs
@@ -117,8 +120,11 @@ export class SupportCodeLibraryBuilder {
         () => this.beforeTestStepHookDefinitionConfigs
       ),
       defineParameterType: this.defineParameterType.bind(this),
-      defineStep,
-      Given: defineStep,
+      defineStep: deprecate(
+        this.defineStep('Unknown', () => this.stepDefinitionConfigs),
+        '`defineStep` is deprecated, use `Given`, `When` or `Then` instead; see https://github.com/cucumber/cucumber-js/issues/2043'
+      ),
+      Given: this.defineStep('Given', () => this.stepDefinitionConfigs),
       setDefaultTimeout: (milliseconds) => {
         this.defaultTimeout = milliseconds
       },
@@ -131,8 +137,8 @@ export class SupportCodeLibraryBuilder {
       setParallelCanAssign: (fn: ParallelAssignmentValidator): void => {
         this.parallelCanAssign = fn
       },
-      Then: defineStep,
-      When: defineStep,
+      Then: this.defineStep('Then', () => this.stepDefinitionConfigs),
+      When: this.defineStep('When', () => this.stepDefinitionConfigs),
     }
   }
 
@@ -142,27 +148,33 @@ export class SupportCodeLibraryBuilder {
   }
 
   defineStep(
-    pattern: DefineStepPattern,
-    options: IDefineStepOptions | Function,
-    code?: Function
-  ): void {
-    if (typeof options === 'function') {
-      code = options
-      options = {}
+    keyword: GherkinStepKeyword,
+    getCollection: () => IStepDefinitionConfig[]
+  ): IDefineStep {
+    return (
+      pattern: DefineStepPattern,
+      options: IDefineStepOptions | Function,
+      code?: Function
+    ) => {
+      if (typeof options === 'function') {
+        code = options
+        options = {}
+      }
+      const { line, uri } = getDefinitionLineAndUri(this.cwd)
+      validateArguments({
+        args: { code, pattern, options },
+        fnName: 'defineStep',
+        location: formatLocation({ line, uri }),
+      })
+      getCollection().push({
+        code,
+        line,
+        options,
+        keyword,
+        pattern,
+        uri,
+      })
     }
-    const { line, uri } = getDefinitionLineAndUri(this.cwd)
-    validateArguments({
-      args: { code, pattern, options },
-      fnName: 'defineStep',
-      location: formatLocation({ line, uri }),
-    })
-    this.stepDefinitionConfigs.push({
-      code,
-      line,
-      options,
-      pattern,
-      uri,
-    })
   }
 
   defineTestCaseHook(
@@ -345,7 +357,7 @@ export class SupportCodeLibraryBuilder {
     const stepDefinitions: StepDefinition[] = []
     const undefinedParameterTypes: messages.UndefinedParameterType[] = []
     this.stepDefinitionConfigs.forEach(
-      ({ code, line, options, pattern, uri }, index) => {
+      ({ code, line, options, keyword, pattern, uri }, index) => {
         let expression
         if (typeof pattern === 'string') {
           try {
@@ -381,6 +393,7 @@ export class SupportCodeLibraryBuilder {
             id: canonicalIds ? canonicalIds[index] : this.newId(),
             line,
             options,
+            keyword,
             pattern,
             unwrappedCode: code,
             uri,

src/support_code_library_builder/index_spec.ts

@@ -99,6 +99,42 @@ describe('supportCodeLibraryBuilder', () => {
         expect(stepDefinition.unwrappedCode).to.eql(step)
       })
     })
+
+    describe('keyword retention', () => {
+      const step = function (): void {} // eslint-disable-line @typescript-eslint/no-empty-function
+
+      beforeEach(() =>
+        supportCodeLibraryBuilder.reset('path/to/project', uuid())
+      )
+
+      it('should record correctly for Given', () => {
+        supportCodeLibraryBuilder.methods.Given('a thing', step)
+        expect(
+          supportCodeLibraryBuilder.finalize().stepDefinitions[0].keyword
+        ).to.eq('Given')
+      })
+
+      it('should record correctly for When', () => {
+        supportCodeLibraryBuilder.methods.When('a thing', step)
+        expect(
+          supportCodeLibraryBuilder.finalize().stepDefinitions[0].keyword
+        ).to.eq('When')
+      })
+
+      it('should record correctly for Then', () => {
+        supportCodeLibraryBuilder.methods.Then('a thing', step)
+        expect(
+          supportCodeLibraryBuilder.finalize().stepDefinitions[0].keyword
+        ).to.eq('Then')
+      })
+
+      it('should record correctly for defineStep', () => {
+        supportCodeLibraryBuilder.methods.defineStep('a thing', step)
+        expect(
+          supportCodeLibraryBuilder.finalize().stepDefinitions[0].keyword
+        ).to.eq('Unknown')
+      })
+    })
   })
 
   describe('After', () => {

src/support_code_library_builder/types.ts

@@ -71,17 +71,19 @@ export interface IParameterTypeDefinition<T> {
   preferForRegexpMatch?: boolean
 }
 
-export interface IDefineSupportCodeMethods {
-  defineParameterType: (options: IParameterTypeDefinition<any>) => void
-  defineStep: (<WorldType = IWorld>(
+export type IDefineStep = (<WorldType = IWorld>(
+  pattern: DefineStepPattern,
+  code: TestStepFunction<WorldType>
+) => void) &
+  (<WorldType = IWorld>(
     pattern: DefineStepPattern,
+    options: IDefineStepOptions,
     code: TestStepFunction<WorldType>
-  ) => void) &
-    (<WorldType = IWorld>(
-      pattern: DefineStepPattern,
-      options: IDefineStepOptions,
-      code: TestStepFunction<WorldType>
-    ) => void)
+  ) => void)
+
+export interface IDefineSupportCodeMethods {
+  defineParameterType: (options: IParameterTypeDefinition<any>) => void
+  defineStep: IDefineStep
   setDefaultTimeout: (milliseconds: number) => void
   setDefinitionFunctionWrapper: (fn: Function) => void
   setParallelCanAssign: (fn: ParallelAssignmentValidator) => void
@@ -132,33 +134,9 @@ export interface IDefineSupportCodeMethods {
     ) => void)
   BeforeAll: ((code: Function) => void) &
     ((options: IDefineTestRunHookOptions, code: Function) => void)
-  Given: (<WorldType = IWorld>(
-    pattern: DefineStepPattern,
-    code: TestStepFunction<WorldType>
-  ) => void) &
-    (<WorldType = IWorld>(
-      pattern: DefineStepPattern,
-      options: IDefineStepOptions,
-      code: TestStepFunction<WorldType>
-    ) => void)
-  Then: (<WorldType = IWorld>(
-    pattern: DefineStepPattern,
-    code: TestStepFunction<WorldType>
-  ) => void) &
-    (<WorldType = IWorld>(
-      pattern: DefineStepPattern,
-      options: IDefineStepOptions,
-      code: TestStepFunction<WorldType>
-    ) => void)
-  When: (<WorldType = IWorld>(
-    pattern: DefineStepPattern,
-    code: TestStepFunction<WorldType>
-  ) => void) &
-    (<WorldType = IWorld>(
-      pattern: DefineStepPattern,
-      options: IDefineStepOptions,
-      code: TestStepFunction<WorldType>
-    ) => void)
+  Given: IDefineStep
+  Then: IDefineStep
+  When: IDefineStep
 }
 
 export interface ISupportCodeCoordinates {

src/wrapper.mjs

@@ -28,6 +28,9 @@ export const Before = cucumber.Before
 export const BeforeAll = cucumber.BeforeAll
 export const BeforeStep = cucumber.BeforeStep
 export const defineParameterType = cucumber.defineParameterType
+/**
+ * @deprecated use `Given`, `When` or `Then` instead; see <https://github.com/cucumber/cucumber-js/issues/2043>
+ */
 export const defineStep = cucumber.defineStep
 export const Given = cucumber.Given
 export const setDefaultTimeout = cucumber.setDefaultTimeout