debt: remove --predictable-ids option (#1801)

* WIP

* fix up testing

* add changelog entry

Author: davidjgoss

CHANGELOG.md

@@ -16,6 +16,7 @@ See the [migration guide](./docs/migration.md) for details of how to migrate fro
 * Drop support for Node.js 10 and 15, add support for Node.js 16
 * Remove deprecated `--retryTagFilter` option (the correct option is `--retry-tag-filter`)
 * Remove `setDefinitionFunctionWrapper` and step definition option `wrapperOptions`
+* Remove `--predictable-ids` option (was only used for internal testing)
 
 ### Added
 

compatibility/cck_spec.ts

@@ -7,7 +7,10 @@ import path from 'path'
 import { PassThrough, pipeline, Writable } from 'stream'
 import { Cli } from '../src'
 import toString from 'stream-to-string'
-import { normalizeMessageOutput } from '../features/support/formatter_output_helpers'
+import {
+  ignorableKeys,
+  normalizeMessageOutput,
+} from '../features/support/formatter_output_helpers'
 import * as messages from '@cucumber/messages'
 import * as messageStreams from '@cucumber/message-streams'
 import util from 'util'
@@ -74,28 +77,7 @@ describe('Cucumber Compatibility Kit', () => {
       )
 
       expect(actualMessages)
-        .excludingEvery([
-          'meta',
-          // sources
-          'uri',
-          'line',
-          // ids
-          'astNodeId',
-          'astNodeIds',
-          'hookId',
-          'id',
-          'pickleId',
-          'pickleStepId',
-          'stepDefinitionIds',
-          'testCaseId',
-          'testCaseStartedId',
-          'testStepId',
-          // time
-          'nanos',
-          'seconds',
-          // errors
-          'message',
-        ])
+        .excludingEvery(ignorableKeys)
         .to.deep.eq(expectedMessages)
     })
   })

features/step_definitions/formatter_steps.ts

@@ -1,6 +1,8 @@
 import { Then } from '../../'
-import { expect } from 'chai'
+import { expect, use } from 'chai'
+import chaiExclude from 'chai-exclude'
 import {
+  ignorableKeys,
   normalizeJsonOutput,
   normalizeMessageOutput,
   stripMetaMessages,
@@ -9,6 +11,8 @@ import fs from 'mz/fs'
 import path from 'path'
 import { World } from '../support/world'
 
+use(chaiExclude)
+
 Then(
   'the message formatter output matches the fixture {string}',
   async function (this: World, filePath: string) {
@@ -18,7 +22,7 @@ Then(
     const fixturePath = path.join(__dirname, '..', 'fixtures', filePath)
     const expected = require(fixturePath) // eslint-disable-line @typescript-eslint/no-var-requires
     try {
-      expect(actual).to.eql(expected)
+      expect(actual).excludingEvery(ignorableKeys).to.deep.eq(expected)
     } catch (e) {
       if (process.env.GOLDEN) {
         await fs.writeFile(

features/support/formatter_output_helpers.ts

@@ -96,3 +96,26 @@ export function normalizeJsonOutput(str: string, cwd: string): IJsonFeature[] {
   })
   return json
 }
+
+export const ignorableKeys = [
+  'meta',
+  // sources
+  'uri',
+  'line',
+  // ids
+  'astNodeId',
+  'astNodeIds',
+  'hookId',
+  'id',
+  'pickleId',
+  'pickleStepId',
+  'stepDefinitionIds',
+  'testCaseId',
+  'testCaseStartedId',
+  'testStepId',
+  // time
+  'nanos',
+  'seconds',
+  // errors
+  'message',
+]

features/support/world.ts

@@ -58,7 +58,6 @@ export class World {
     const messageFilename = 'message.ndjson'
     const args = ['node', executablePath].concat(inputArgs, [
       '--backtrace',
-      '--predictable-ids',
       '--format',
       `message:${messageFilename}`,
     ])

src/cli/argv_parser.ts

@@ -32,7 +32,6 @@ export interface IParsedArgvOptions {
   name: string[]
   order: string
   parallel: number
-  predictableIds: boolean
   profile: string[]
   publish: boolean
   publishQuiet: boolean
@@ -167,11 +166,6 @@ const ArgvParser = {
         (val) => ArgvParser.validateCountOption(val, '--parallel'),
         0
       )
-      .option(
-        '--predictable-ids',
-        'Use predictable ids in messages (option ignored if using parallel)',
-        false
-      )
       .option(
         '--publish',
         'Publish a report to https://reports.cucumber.io',

src/cli/configuration_builder.ts

@@ -27,7 +27,6 @@ export interface IConfiguration {
   order: string
   parallel: number
   pickleFilterOptions: IPickleFilterOptions
-  predictableIds: boolean
   profiles: string[]
   runtimeOptions: IRuntimeOptions
   shouldExitImmediately: boolean
@@ -95,11 +94,9 @@ export default class ConfigurationBuilder {
         names: this.options.name,
         tagExpression: this.options.tags,
       },
-      predictableIds: this.options.predictableIds,
       profiles: this.options.profile,
       runtimeOptions: {
         dryRun: this.options.dryRun,
-        predictableIds: this.options.predictableIds,
         failFast: this.options.failFast,
         filterStacktraces: !this.options.backtrace,
         retry: this.options.retry,

src/cli/configuration_builder_spec.ts

@@ -45,12 +45,10 @@ describe('Configuration', () => {
           tagExpression: '',
         },
         profiles: [],
-        predictableIds: false,
         runtimeOptions: {
           dryRun: false,
           failFast: false,
           filterStacktraces: true,
-          predictableIds: false,
           retry: 0,
           retryTagFilter: '',
           strict: true,

src/cli/index.ts

@@ -33,7 +33,7 @@ import { pathToFileURL } from 'url'
 
 // eslint-disable-next-line @typescript-eslint/no-var-requires
 const { importer } = require('../importer')
-const { incrementing, uuid } = IdGenerator
+const { uuid } = IdGenerator
 
 export interface ICliRunResult {
   shouldExitImmediately: boolean
@@ -182,10 +182,7 @@ export default class Cli {
       this.stdout.write(I18n.getKeywords(configuration.listI18nKeywordsFor))
       return { shouldExitImmediately: true, success: true }
     }
-    const newId =
-      configuration.predictableIds && configuration.parallel <= 1
-        ? incrementing()
-        : uuid()
+    const newId = uuid()
     const supportCodeLibrary = await this.getSupportCodeLibrary({
       newId,
       supportCodePaths: configuration.supportCodePaths,

src/runtime/index.ts

@@ -10,11 +10,7 @@ import { EventEmitter } from 'events'
 import { ISupportCodeLibrary } from '../support_code_library_builder/types'
 import TestRunHookDefinition from '../models/test_run_hook_definition'
 import { doesHaveValue, valueOrDefault } from '../value_checker'
-import {
-  ITestRunStopwatch,
-  PredictableTestRunStopwatch,
-  RealTestRunStopwatch,
-} from './stopwatch'
+import { ITestRunStopwatch, RealTestRunStopwatch } from './stopwatch'
 import { assembleTestCases } from './assemble_test_cases'
 
 export interface INewRuntimeOptions {
@@ -28,7 +24,6 @@ export interface INewRuntimeOptions {
 
 export interface IRuntimeOptions {
   dryRun: boolean
-  predictableIds: boolean
   failFast: boolean
   filterStacktraces: boolean
   retry: number
@@ -58,9 +53,7 @@ export default class Runtime {
   }: INewRuntimeOptions) {
     this.eventBroadcaster = eventBroadcaster
     this.eventDataCollector = eventDataCollector
-    this.stopwatch = options.predictableIds
-      ? new PredictableTestRunStopwatch()
-      : new RealTestRunStopwatch()
+    this.stopwatch = new RealTestRunStopwatch()
     this.newId = newId
     this.options = options
     this.pickleIds = pickleIds

src/runtime/parallel/coordinator.ts

@@ -8,11 +8,7 @@ import { IRuntimeOptions } from '..'
 import { ISupportCodeLibrary } from '../../support_code_library_builder/types'
 import { ICoordinatorReport, IWorkerCommand } from './command_types'
 import { doesHaveValue } from '../../value_checker'
-import {
-  ITestRunStopwatch,
-  PredictableTestRunStopwatch,
-  RealTestRunStopwatch,
-} from '../stopwatch'
+import { ITestRunStopwatch, RealTestRunStopwatch } from '../stopwatch'
 import { assembleTestCases, IAssembledTestCases } from '../assemble_test_cases'
 import { IdGenerator } from '@cucumber/messages'
 
@@ -66,9 +62,7 @@ export default class Coordinator {
     this.cwd = cwd
     this.eventBroadcaster = eventBroadcaster
     this.eventDataCollector = eventDataCollector
-    this.stopwatch = options.predictableIds
-      ? new PredictableTestRunStopwatch()
-      : new RealTestRunStopwatch()
+    this.stopwatch = new RealTestRunStopwatch()
     this.options = options
     this.newId = newId
     this.supportCodeLibrary = supportCodeLibrary

src/runtime/parallel/worker.ts

@@ -16,7 +16,7 @@ import TestRunHookDefinition from '../../models/test_run_hook_definition'
 import { ISupportCodeLibrary } from '../../support_code_library_builder/types'
 import { doesHaveValue, valueOrDefault } from '../../value_checker'
 import { IRuntimeOptions } from '../index'
-import { PredictableTestRunStopwatch, RealTestRunStopwatch } from '../stopwatch'
+import { RealTestRunStopwatch } from '../stopwatch'
 import { duration } from 'durations'
 import { pathToFileURL } from 'url'
 
@@ -126,9 +126,7 @@ export default class Worker {
     retries,
     skip,
   }: IWorkerCommandRun): Promise<void> {
-    const stopwatch = this.options.predictableIds
-      ? new PredictableTestRunStopwatch()
-      : new RealTestRunStopwatch()
+    const stopwatch = new RealTestRunStopwatch()
     stopwatch.from(duration(elapsed))
     const testCaseRunner = new TestCaseRunner({
       eventBroadcaster: this.eventBroadcaster,

test/runtime_helpers.ts

@@ -12,7 +12,6 @@ export function buildOptions(
 ): IRuntimeOptions {
   return {
     dryRun: false,
-    predictableIds: false,
     failFast: false,
     filterStacktraces: false,
     retry: 0,