From 7e15c746bca34a4d293b79544458558ebbe9fb83 Mon Sep 17 00:00:00 2001 From: Teodor Todorov Date: Mon, 5 Feb 2024 16:52:16 +0100 Subject: [PATCH 1/5] Fix bug --- src/commands/synthetics/utils/public.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/commands/synthetics/utils/public.ts b/src/commands/synthetics/utils/public.ts index 2549eea52..722e29778 100644 --- a/src/commands/synthetics/utils/public.ts +++ b/src/commands/synthetics/utils/public.ts @@ -54,7 +54,7 @@ import { } from './internal' const POLLING_INTERVAL = 5000 // In ms -const PUBLIC_ID_REGEX = /^[\d\w]{3}-[\d\w]{3}-[\d\w]{3}$/ +const PUBLIC_ID_REGEX = /\b[\d\w]{3}-[\d\w]{3}-[\d\w]{3}\b/ const TEMPLATE_REGEX = /{{\s*([^{}]*?)\s*}}/g export const readableOperation: {[key in Operator]: string} = { @@ -644,7 +644,11 @@ export const getTestAndOverrideConfig = async ( summary: InitialSummary, isTunnelEnabled?: boolean ): Promise => { - const normalizedId = PUBLIC_ID_REGEX.test(id) ? id : id.substring(id.lastIndexOf('/') + 1) + const normalizedId = id.match(PUBLIC_ID_REGEX)?.[0] + + if (normalizedId === undefined) { + throw new CriticalError('INVALID_CONFIG', `No valid public ID found in: \`${id}\``) + } const testResult = await getTest(api, {config, id: normalizedId, suite}) if ('errorMessage' in testResult) { From c4b9ee2e1ff17519f7e121ab5538c70b5f6bcd0e Mon Sep 17 00:00:00 2001 From: Teodor Todorov Date: Mon, 5 Feb 2024 17:49:53 +0100 Subject: [PATCH 2/5] Add tests to make sure bug is fixed --- .../synthetics/__tests__/utils/public.test.ts | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/commands/synthetics/__tests__/utils/public.test.ts b/src/commands/synthetics/__tests__/utils/public.test.ts index 2d46183b6..9e1a123be 100644 --- a/src/commands/synthetics/__tests__/utils/public.test.ts +++ b/src/commands/synthetics/__tests__/utils/public.test.ts @@ -420,6 +420,27 @@ describe('utils', () => { ).rejects.toThrow('Failed to get test: could not query https://app.datadoghq.com/example\nForbidden\n') }) + test('Passes when public ID is valid', async () => { + const axiosMock = jest.spyOn(axios, 'create') + axiosMock.mockImplementation((() => (e: any) => { + return {data: {subtype: 'http', public_id: '123-456-789'}} + }) as any) + + const triggerConfig = {suite: 'Suite 1', config: {}, id: '123-456-789'} + expect(await utils.getTestAndOverrideConfig(api, triggerConfig, mockReporter, getSummary())).toEqual( + expect.objectContaining({test: expect.objectContaining({public_id: '123-456-789', subtype: 'http'})}) + ) + }) + + test('Fails when public ID is NOT valid', async () => { + const expectedError = new CiError('INVALID_CONFIG', `No valid public ID found in: \`a123-456-789\``) + + const triggerConfig = {suite: 'Suite 1', config: {}, id: 'a123-456-789'} + await expect(utils.getTestAndOverrideConfig(api, triggerConfig, mockReporter, getSummary())).rejects.toThrow( + expectedError + ) + }) + test('Passes when the tunnel is enabled for HTTP test', async () => { const axiosMock = jest.spyOn(axios, 'create') axiosMock.mockImplementation((() => (e: any) => { From 36c25dbf00faa628eba24341813261b13cfa11c7 Mon Sep 17 00:00:00 2001 From: Teodor Todorov Date: Tue, 6 Feb 2024 11:59:43 +0100 Subject: [PATCH 3/5] Make other tests have correctly formated public-ids --- .../__tests__/run-tests-lib.test.ts | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/commands/synthetics/__tests__/run-tests-lib.test.ts b/src/commands/synthetics/__tests__/run-tests-lib.test.ts index ce30729a5..5fe8e5b44 100644 --- a/src/commands/synthetics/__tests__/run-tests-lib.test.ts +++ b/src/commands/synthetics/__tests__/run-tests-lib.test.ts @@ -53,14 +53,14 @@ describe('run-test', () => { runTests.executeTests(mockReporter, { ...ciConfig, global: userConfigOverride, - publicIds: ['public-id-1', 'public-id-2'], + publicIds: ['aaa-aaa-aaa', 'bbb-bbb-bbb'], }) ).rejects.toThrow() expect(getTestsToTriggersMock).toHaveBeenCalledWith( apiHelper, expect.arrayContaining([ - expect.objectContaining({id: 'public-id-1', config: userConfigOverride}), - expect.objectContaining({id: 'public-id-2', config: userConfigOverride}), + expect.objectContaining({id: 'aaa-aaa-aaa', config: userConfigOverride}), + expect.objectContaining({id: 'bbb-bbb-bbb', config: userConfigOverride}), ]), expect.anything(), false, @@ -103,14 +103,14 @@ describe('run-test', () => { runTests.executeTests(mockReporter, { ...ciConfig, ...partialCIConfig, - publicIds: ['public-id-1', 'public-id-2'], + publicIds: ['aaa-aaa-aaa', 'bbb-bbb-bbb'], }) ).rejects.toThrow(new CiError('NO_TESTS_TO_RUN')) expect(getTestsToTriggersMock).toHaveBeenCalledWith( apiHelper, expect.arrayContaining([ - expect.objectContaining({id: 'public-id-1', config: expectedOverriddenConfig}), - expect.objectContaining({id: 'public-id-2', config: expectedOverriddenConfig}), + expect.objectContaining({id: 'aaa-aaa-aaa', config: expectedOverriddenConfig}), + expect.objectContaining({id: 'bbb-bbb-bbb', config: expectedOverriddenConfig}), ]), expect.anything(), false, @@ -137,14 +137,14 @@ describe('run-test', () => { runTests.executeTests(mockReporter, { ...ciConfig, global: configOverride, - publicIds: ['public-id-1', 'public-id-2'], + publicIds: ['aaa-aaa-aaa', 'bbb-bbb-bbb'], }) ).rejects.toThrow(new CiError('NO_TESTS_TO_RUN')) expect(getTestsToTriggersMock).toHaveBeenCalledWith( apiHelper, expect.arrayContaining([ - expect.objectContaining({id: 'public-id-1', config: configOverride}), - expect.objectContaining({id: 'public-id-2', config: configOverride}), + expect.objectContaining({id: 'aaa-aaa-aaa', config: configOverride}), + expect.objectContaining({id: 'bbb-bbb-bbb', config: configOverride}), ]), expect.anything(), false, @@ -172,15 +172,15 @@ describe('run-test', () => { runTests.executeTests(mockReporter, { ...ciConfig, global: configOverride, - publicIds: ['public-id-1', 'public-id-2'], + publicIds: ['aaa-aaa-aaa', 'bbb-bbb-bbb'], tunnel: true, }) ).rejects.toThrow(new CiError('NO_TESTS_TO_RUN')) expect(getTestsToTriggersMock).toHaveBeenCalledWith( apiHelper, expect.arrayContaining([ - expect.objectContaining({id: 'public-id-1', config: configOverride}), - expect.objectContaining({id: 'public-id-2', config: configOverride}), + expect.objectContaining({id: 'aaa-aaa-aaa', config: configOverride}), + expect.objectContaining({id: 'bbb-bbb-bbb', config: configOverride}), ]), expect.anything(), false, @@ -251,7 +251,7 @@ describe('run-test', () => { } jest.spyOn(api, 'getApiHelper').mockImplementation(() => apiHelper as any) await expect( - runTests.executeTests(mockReporter, {...ciConfig, publicIds: ['public-id-1'], tunnel: true}) + runTests.executeTests(mockReporter, {...ciConfig, publicIds: ['aaa-aaa-aaa'], tunnel: true}) ).rejects.toThrow( new CriticalError( error, @@ -278,7 +278,7 @@ describe('run-test', () => { jest.spyOn(api, 'getApiHelper').mockImplementation(() => apiHelper as any) await expect( - runTests.executeTests(mockReporter, {...ciConfig, publicIds: ['public-id-1', 'public-id-2'], tunnel: true}) + runTests.executeTests(mockReporter, {...ciConfig, publicIds: ['aaa-aaa-aaa', 'bbb-bbb-bbb'], tunnel: true}) ).rejects.toThrow(new CriticalError('UNAVAILABLE_TUNNEL_CONFIG', 'Server Error')) }) @@ -366,7 +366,7 @@ describe('run-test', () => { jest.spyOn(api, 'getApiHelper').mockImplementation(() => apiHelper as any) await expect( - runTests.executeTests(mockReporter, {...ciConfig, publicIds: ['public-id-1', 'public-id-2'], tunnel: true}) + runTests.executeTests(mockReporter, {...ciConfig, publicIds: ['aaa-aaa-aaa', 'bbb-bbb-bbb'], tunnel: true}) ).rejects.toThrow( new CriticalError( 'TRIGGER_TESTS_FAILED', @@ -416,7 +416,7 @@ describe('run-test', () => { runTests.executeTests(mockReporter, { ...ciConfig, failOnCriticalErrors: true, - publicIds: ['public-id-1', 'public-id-2'], + publicIds: ['aaa-aaa-aaa', 'bbb-bbb-bbb'], tunnel: true, }) ).rejects.toThrow( From 2833a60b1cba66a9225a5f6ad7c0662a4a849ba2 Mon Sep 17 00:00:00 2001 From: Teodor Todorov Date: Tue, 6 Feb 2024 12:03:23 +0100 Subject: [PATCH 4/5] make the regex even more precise --- src/commands/synthetics/utils/public.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commands/synthetics/utils/public.ts b/src/commands/synthetics/utils/public.ts index 722e29778..f2f5cf3ef 100644 --- a/src/commands/synthetics/utils/public.ts +++ b/src/commands/synthetics/utils/public.ts @@ -54,7 +54,7 @@ import { } from './internal' const POLLING_INTERVAL = 5000 // In ms -const PUBLIC_ID_REGEX = /\b[\d\w]{3}-[\d\w]{3}-[\d\w]{3}\b/ +const PUBLIC_ID_REGEX = /\b[a-z0-9]{3}-[a-z0-9]{3}-[a-z0-9]{3}\b/ const TEMPLATE_REGEX = /{{\s*([^{}]*?)\s*}}/g export const readableOperation: {[key in Operator]: string} = { From 23117c08e18f794fa08e1d9da974005fd86e09d8 Mon Sep 17 00:00:00 2001 From: Teodor Todorov Date: Tue, 6 Feb 2024 12:03:50 +0100 Subject: [PATCH 5/5] improvment from comment --- src/commands/synthetics/utils/public.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commands/synthetics/utils/public.ts b/src/commands/synthetics/utils/public.ts index f2f5cf3ef..4bb1b22de 100644 --- a/src/commands/synthetics/utils/public.ts +++ b/src/commands/synthetics/utils/public.ts @@ -646,7 +646,7 @@ export const getTestAndOverrideConfig = async ( ): Promise => { const normalizedId = id.match(PUBLIC_ID_REGEX)?.[0] - if (normalizedId === undefined) { + if (!normalizedId) { throw new CriticalError('INVALID_CONFIG', `No valid public ID found in: \`${id}\``) }