[SYNTH-12982] Fix the usage of variableStrings (#1354)

* Fix the usage of variableStrings

* update doc

* Add missing tests

* format

* fix nits

* remove comment

src/commands/synthetics/__tests__/cli.test.ts

@@ -13,7 +13,7 @@ import {
 } from '../interfaces'
 import {DEFAULT_COMMAND_CONFIG, DEFAULT_POLLING_TIMEOUT, RunTestsCommand} from '../run-tests-command'
 import {DEFAULT_UPLOAD_COMMAND_CONFIG, UploadApplicationCommand} from '../upload-application-command'
-import {toBoolean, toNumber, toExecutionRule, toStringObject} from '../utils/internal'
+import {toBoolean, toNumber, toExecutionRule, toStringMap} from '../utils/internal'
 import * as utils from '../utils/public'
 
 import {getApiTest, getAxiosHttpError, getTestSuite, mockApi, mockTestTriggerResponse} from './fixtures'
@@ -92,6 +92,7 @@ describe('run-test', () => {
  DATADOG_SYNTHETICS_OVERRIDE_START_URL: 'startUrl',
  DATADOG_SYNTHETICS_OVERRIDE_START_URL_SUBSTITUTION_REGEX: 'startUrlSubstitutionRegex',
  DATADOG_SYNTHETICS_OVERRIDE_TEST_TIMEOUT: '42',
+ DATADOG_SYNTHETICS_OVERRIDE_VARIABLES: "{'var1': 'value1', 'var2': 'value2'}",
  }
 
  process.env = overrideEnv
@@ -120,7 +121,7 @@ describe('run-test', () => {
  deviceIds: overrideEnv.DATADOG_SYNTHETICS_OVERRIDE_DEVICE_IDS.split(';'),
  executionRule: toExecutionRule(overrideEnv.DATADOG_SYNTHETICS_OVERRIDE_EXECUTION_RULE),
  followRedirects: toBoolean(overrideEnv.DATADOG_SYNTHETICS_OVERRIDE_FOLLOW_REDIRECTS),
- headers: toStringObject(overrideEnv.DATADOG_SYNTHETICS_OVERRIDE_HEADERS),
+ headers: toStringMap(overrideEnv.DATADOG_SYNTHETICS_OVERRIDE_HEADERS),
  mobileApplicationVersion: overrideEnv.DATADOG_SYNTHETICS_OVERRIDE_MOBILE_APPLICATION_VERSION,
  pollingTimeout: DEFAULT_POLLING_TIMEOUT,
  resourceUrlSubstitutionRegexes: overrideEnv.DATADOG_SYNTHETICS_OVERRIDE_RESOURCE_URL_SUBSTITUTION_REGEXES?.split(
@@ -133,6 +134,7 @@ describe('run-test', () => {
  startUrl: overrideEnv.DATADOG_SYNTHETICS_OVERRIDE_START_URL,
  startUrlSubstitutionRegex: overrideEnv.DATADOG_SYNTHETICS_OVERRIDE_START_URL_SUBSTITUTION_REGEX,
  testTimeout: toNumber(overrideEnv.DATADOG_SYNTHETICS_OVERRIDE_TEST_TIMEOUT),
+ variables: toStringMap(overrideEnv.DATADOG_SYNTHETICS_OVERRIDE_VARIABLES),
  },
  failOnCriticalErrors: toBoolean(overrideEnv.DATADOG_SYNTHETICS_FAIL_ON_CRITICAL_ERRORS),
  failOnMissingTests: toBoolean(overrideEnv.DATADOG_SYNTHETICS_FAIL_ON_MISSING_TESTS),
@@ -236,7 +238,8 @@ describe('run-test', () => {
  subdomain: 'new-sub-domain',
  testSearchQuery: 'a-search-query',
  tunnel: true,
- variableStrings: ['key=value'],
+ // TODO SYNTH-12989: Clean up deprecated `variableStrings` in favor of `variables` in `defaultTestOverrides`.
+ variableStrings: ['var3=value3', 'var4=value4'],
  }
  const defaultTestOverrides: UserConfigOverride = {
  allowInsecureCertificates: true,
@@ -263,6 +266,7 @@ describe('run-test', () => {
  startUrl: 'startUrl',
  startUrlSubstitutionRegex: 'startUrlSubstitutionRegex',
  testTimeout: 42,
+ variables: {var1: 'value1', var2: 'value2'},
  }
 
  const command = createCommand(RunTestsCommand)
@@ -281,6 +285,8 @@ describe('run-test', () => {
  command['subdomain'] = overrideCLI.subdomain
  command['tunnel'] = overrideCLI.tunnel
  command['testSearchQuery'] = overrideCLI.testSearchQuery
+ // TODO SYNTH-12989: Clean up deprecated `variableStrings` in favor of `variables` in `defaultTestOverrides`.
+ command['variableStrings'] = overrideCLI.variableStrings
  command['overrides'] = [
  `allowInsecureCertificates=${defaultTestOverrides.allowInsecureCertificates}`,
  `basicAuth.password=${defaultTestOverrides.basicAuth?.password}`,
@@ -303,6 +309,8 @@ describe('run-test', () => {
  `testTimeout=${defaultTestOverrides.testTimeout}`,
  'resourceUrlSubstitutionRegexes=regex1',
  'resourceUrlSubstitutionRegexes=regex42',
+ `variables.var1=${defaultTestOverrides.variables?.var1}`,
+ `variables.var2=${defaultTestOverrides.variables?.var2}`,
  ]
 
  await command['resolveConfig']()
@@ -345,6 +353,7 @@ describe('run-test', () => {
  startUrl: 'startUrl',
  startUrlSubstitutionRegex: 'startUrlSubstitutionRegex',
  testTimeout: 42,
+ variables: {var1: 'value1', var2: 'value2'},
  },
  publicIds: ['ran-dom-id'],
  subdomain: 'new-sub-domain',
@@ -353,6 +362,20 @@ describe('run-test', () => {
  })
  })
 
+ // TODO SYNTH-12989: Clean up deprecated `variableStrings` in favor of `variables` in `defaultTestOverrides`.
+ test("CLI parameter '--variable' still works (deprecated)", async () => {
+ const command = createCommand(RunTestsCommand)
+ command['variableStrings'] = ['var1=value1', 'var2=value2']
+ await command['resolveConfig']()
+ expect(command['config']).toEqual({
+ ...DEFAULT_COMMAND_CONFIG,
+ defaultTestOverrides: {
+ pollingTimeout: DEFAULT_POLLING_TIMEOUT,
+ variables: {var1: 'value1', var2: 'value2'},
+ },
+ })
+ })
+
  // We have 2 code paths that handle different levels of configuration overrides:
  // 1) config file (configuration of datadog-ci) < ENV (environment variables) < CLI (command flags)
  // 2) global (global config object, aka. `config.global`) < ENV (environment variables) < test file (test configuration)

src/commands/synthetics/__tests__/run-tests-lib.test.ts

@@ -70,7 +70,7 @@ describe('run-test', () => {
  selectiveRerun: false,
  subdomain: 'app',
  tunnel: false,
- variableStrings: [],
+ variableStrings: [], // deprecated
  })
  ).rejects.toThrow(new CiError('NO_TESTS_TO_RUN'))
  })
@@ -99,6 +99,7 @@ describe('run-test', () => {
  selectiveRerun: false,
  subdomain: 'app',
  tunnel: false,
+ // TODO SYNTH-12989: Clean up deprecated `variableStrings`
  variableStrings: [],
  })
  ).rejects.toThrow(new CiError('NO_TESTS_TO_RUN'))

src/commands/synthetics/__tests__/utils/internal.test.ts

@@ -5,7 +5,7 @@ import {
  toBoolean,
  toExecutionRule,
  toNumber,
- toStringObject,
+ toStringMap,
  validateAndParseOverrides,
 } from '../../utils/internal'
 
@@ -125,7 +125,7 @@ describe('utils', () => {
  ]
 
  test.each(cases)('toObject(%s) should return %s', (input, expectedOutput) => {
- expect(toStringObject(input)).toEqual(expectedOutput)
+ expect(toStringMap(input)).toEqual(expectedOutput)
  })
  })
 

src/commands/synthetics/interfaces.ts

@@ -483,6 +483,8 @@ export interface RunTestsCommandConfig extends SyntheticsCIConfig {
  subdomain: string
  testSearchQuery?: string
  tunnel: boolean
+ // TODO SYNTH-12989: Clean up deprecated `variableStrings` in favor of `variables` in `defaultTestOverrides`.
+ /** @deprecated This property is deprecated, please use `variables` inside of `defaultTestOverrides`. */
  variableStrings: string[]
 }
 

src/commands/synthetics/run-tests-command.ts

@@ -12,7 +12,7 @@ import {MainReporter, Reporter, Result, RunTestsCommandConfig, Summary} from './
 import {DefaultReporter} from './reporters/default'
 import {JUnitReporter} from './reporters/junit'
 import {executeTests} from './run-tests-lib'
-import {toBoolean, toNumber, toExecutionRule, validateAndParseOverrides, toStringObject} from './utils/internal'
+import {toBoolean, toNumber, toExecutionRule, validateAndParseOverrides, toStringMap} from './utils/internal'
 import {
  getExitReason,
  getOrgSettings,
@@ -50,6 +50,7 @@ export const DEFAULT_COMMAND_CONFIG: RunTestsCommandConfig = {
  subdomain: 'app',
  testSearchQuery: '',
  tunnel: false,
+ // TODO SYNTH-12989: Clean up deprecated `variableStrings` in favor of `variables` in `defaultTestOverrides`.
  variableStrings: [],
 }
 
@@ -96,8 +97,10 @@ export class RunTestsCommand extends Command {
  private appKey = Option.String('--appKey', {description: 'The application key used to query the Datadog API.'})
  private datadogSite = Option.String('--datadogSite', {description: 'The Datadog instance to which request is sent.'})
  // TODO SYNTH-12989: Clean up deprecated `--deviceIds` in favor of `--override deviceIds="dev1;dev2;..."`
+ /** @deprecated This is deprecated, please use `--override deviceIds="dev1;dev2;..."` instead. */
  private deviceIds = Option.Array('--deviceIds', {
- description: 'Override the mobile device(s) to run your mobile test.',
+ description:
+ '**DEPRECATED** Override the mobile device(s) to run your mobile test. Use `--override deviceIds="dev1;dev2;..."` instead.',
  })
  private failOnCriticalErrors = Option.Boolean('--failOnCriticalErrors', {
  description:
@@ -143,7 +146,11 @@ export class RunTestsCommand extends Command {
  private tunnel = Option.Boolean('-t,--tunnel', {
  description: `Use the ${$3('Continuous Testing Tunnel')} to execute your test batch.`,
  })
- private variableStrings = Option.Array('-v,--variable', {description: 'Pass a variable override.'})
+ // TODO SYNTH-12989: Clean up deprecated `variableStrings` in favor of `variables` in `defaultTestOverrides`.
+ /** @deprecated This is deprecated, please use `--override variables.NAME=VALUE` instead. */
+ private variableStrings = Option.Array('-v,--variable', {
+ description: '**DEPRECATED** Pass a variable override. Use `--override variables.NAME=VALUE` instead.',
+ })
 
  private reporter!: MainReporter
  private config: RunTestsCommandConfig = JSON.parse(JSON.stringify(DEFAULT_COMMAND_CONFIG)) // Deep copy to avoid mutation
@@ -286,7 +293,7 @@ export class RunTestsCommand extends Command {
  deviceIds: process.env.DATADOG_SYNTHETICS_OVERRIDE_DEVICE_IDS?.split(';'),
  executionRule: toExecutionRule(process.env.DATADOG_SYNTHETICS_OVERRIDE_EXECUTION_RULE),
  followRedirects: toBoolean(process.env.DATADOG_SYNTHETICS_OVERRIDE_FOLLOW_REDIRECTS),
- headers: toStringObject(process.env.DATADOG_SYNTHETICS_OVERRIDE_HEADERS),
+ headers: toStringMap(process.env.DATADOG_SYNTHETICS_OVERRIDE_HEADERS),
  // TODO SYNTH-12989: Clean up `locations` that should only be part of the testOverrides
  locations:
  process.env.DATADOG_SYNTHETICS_OVERRIDE_LOCATIONS?.split(';') ??
@@ -299,6 +306,7 @@ export class RunTestsCommand extends Command {
  startUrl: process.env.DATADOG_SYNTHETICS_OVERRIDE_START_URL,
  startUrlSubstitutionRegex: process.env.DATADOG_SYNTHETICS_OVERRIDE_START_URL_SUBSTITUTION_REGEX,
  testTimeout: toNumber(process.env.DATADOG_SYNTHETICS_OVERRIDE_TEST_TIMEOUT),
+ variables: toStringMap(process.env.DATADOG_SYNTHETICS_OVERRIDE_VARIABLES),
  })
  )
 
@@ -366,7 +374,9 @@ export class RunTestsCommand extends Command {
  startUrl: validatedOverrides.startUrl,
  startUrlSubstitutionRegex: validatedOverrides.startUrlSubstitutionRegex,
  testTimeout: validatedOverrides.testTimeout,
- variables: parseVariablesFromCli(this.variableStrings, (log) => this.reporter.log(log)),
+ // TODO SYNTH-12989: Clean up deprecated `variableStrings` in favor of `variables` in `defaultTestOverrides`.
+ variables:
+ validatedOverrides.variables ?? parseVariablesFromCli(this.variableStrings, (log) => this.reporter.log(log)),
  })
  )
 

src/commands/synthetics/utils/internal.ts

@@ -90,7 +90,7 @@ export const toExecutionRule = (env: string | undefined): ExecutionRule | undefi
  return undefined
 }
 
-export const toStringObject = (env: string | undefined): {[key: string]: string} | undefined => {
+export const toStringMap = (env: string | undefined): StringMap | undefined => {
  if (env === undefined) {
  return undefined
  }
@@ -106,13 +106,15 @@ export const toStringObject = (env: string | undefined): {[key: string]: string}
  }
  }
 
- return parsed as {[key: string]: string}
+ return parsed as StringMap
  }
  } catch (error) {
  return undefined
  }
 }
 
+type StringMap = {[key: string]: string}
+
 type AccumulatorBaseConfigOverride = Omit<
  UserConfigOverride,
  | 'retry'
@@ -122,7 +124,6 @@ type AccumulatorBaseConfigOverride = Omit<
  | 'mobileApplicationVersion'
  | 'mobileApplicationVersionFilePath'
  | 'tunnel'
- | 'variables'
 > & {
  retry?: Partial<RetryConfig>
  basicAuth?: Partial<BasicAuthCredentials>
@@ -253,11 +254,12 @@ export const validateAndParseOverrides = (overrides: string[] | undefined): Accu
  }
  break
 
- // Convert to {[key: string]: string}
+ // Convert to StringMap
  case 'headers':
+ case 'variables':
  if (subKey) {
- acc['headers'] = acc['headers'] ?? {}
- acc['headers'][subKey] = value
+ acc[key] = acc[key] ?? {}
+ ;(acc[key] as StringMap)[subKey] = value
  } else {
  throw new Error(`No subkey found for ${key}`)
  }

src/commands/synthetics/utils/public.ts

@@ -646,6 +646,7 @@ export const retry = async <T, E extends Error>(
  return trier()
 }
 
+// TODO SYNTH-12989: Clean up deprecated `variableStrings` in favor of `variables` in `defaultTestOverrides`.
 export const parseVariablesFromCli = (
  variableArguments: string[] = [],
  logFunction: (log: string) => void