From bd76b5909cdfe3b20f64af3d204a2b3e6de5f129 Mon Sep 17 00:00:00 2001 From: droctothorpe Date: Tue, 13 Aug 2024 10:56:08 -0400 Subject: [PATCH] Update frontend unit tests Signed-off-by: droctothorpe --- frontend/server/workflow-helper.test.ts | 37 ++++++++++++++----------- frontend/server/workflow-helper.ts | 2 +- frontend/src/lib/Apis.test.ts | 17 ++++++++++-- frontend/src/lib/Apis.ts | 10 ++++--- 4 files changed, 43 insertions(+), 23 deletions(-) diff --git a/frontend/server/workflow-helper.test.ts b/frontend/server/workflow-helper.test.ts index ae21378b46cb..b7cecb9bbc8c 100644 --- a/frontend/server/workflow-helper.test.ts +++ b/frontend/server/workflow-helper.test.ts @@ -39,40 +39,41 @@ describe('workflow-helper', () => { describe('composePodLogsStreamHandler', () => { it('returns the stream from the default handler if there is no errors.', async () => { const defaultStream = new PassThrough(); - const defaultHandler = jest.fn((_podName: string, _namespace?: string) => + const defaultHandler = jest.fn((_podName: string, _createdAt: string, _namespace?: string) => Promise.resolve(defaultStream), ); - const stream = await composePodLogsStreamHandler(defaultHandler)('podName', 'namespace'); - expect(defaultHandler).toBeCalledWith('podName', 'namespace'); + const stream = await composePodLogsStreamHandler(defaultHandler)('podName', '2024-08-13', 'namespace'); + expect(defaultHandler).toBeCalledWith('podName', '2024-08-13', 'namespace'); expect(stream).toBe(defaultStream); }); it('returns the stream from the fallback handler if there is any error.', async () => { const fallbackStream = new PassThrough(); - const defaultHandler = jest.fn((_podName: string, _namespace?: string) => + const defaultHandler = jest.fn((_podName: string, _createdAt: string, _namespace?: string) => Promise.reject('unknown error'), ); - const fallbackHandler = jest.fn((_podName: string, _namespace?: string) => + const fallbackHandler = jest.fn((_podName: string, _createdAt: string, _namespace?: string) => Promise.resolve(fallbackStream), ); const stream = await composePodLogsStreamHandler(defaultHandler, fallbackHandler)( 'podName', + '2024-08-13', 'namespace', ); - expect(defaultHandler).toBeCalledWith('podName', 'namespace'); - expect(fallbackHandler).toBeCalledWith('podName', 'namespace'); + expect(defaultHandler).toBeCalledWith('podName', '2024-08-13', 'namespace'); + expect(fallbackHandler).toBeCalledWith('podName', '2024-08-13', 'namespace'); expect(stream).toBe(fallbackStream); }); it('throws error if both handler and fallback fails.', async () => { - const defaultHandler = jest.fn((_podName: string, _namespace?: string) => + const defaultHandler = jest.fn((_podName: string, _createdAt: string, _namespace?: string) => Promise.reject('unknown error for default'), ); - const fallbackHandler = jest.fn((_podName: string, _namespace?: string) => + const fallbackHandler = jest.fn((_podName: string, _createdAt: string, _namespace?: string) => Promise.reject('unknown error for fallback'), ); await expect( - composePodLogsStreamHandler(defaultHandler, fallbackHandler)('podName', 'namespace'), + composePodLogsStreamHandler(defaultHandler, fallbackHandler)('podName', '2024-08-13', 'namespace'), ).rejects.toEqual('unknown error for fallback'); }); }); @@ -82,7 +83,7 @@ describe('workflow-helper', () => { const mockedGetPodLogs: jest.Mock = getPodLogs as any; mockedGetPodLogs.mockResolvedValueOnce('pod logs'); - const stream = await getPodLogsStreamFromK8s('podName', 'namespace'); + const stream = await getPodLogsStreamFromK8s('podName', '', 'namespace'); expect(mockedGetPodLogs).toBeCalledWith('podName', 'namespace', 'main'); expect(stream.read().toString()).toBe('pod logs'); }); @@ -101,10 +102,10 @@ describe('workflow-helper', () => { client, key: 'folder/key', }; - const createRequest = jest.fn((_podName: string, _namespace?: string) => + const createRequest = jest.fn((_podName: string, _createdAt: string, _namespace?: string) => Promise.resolve(configs), ); - const stream = await toGetPodLogsStream(createRequest)('podName', 'namespace'); + const stream = await toGetPodLogsStream(createRequest)('podName', '2024-08-13', 'namespace'); expect(mockedClientGetObject).toBeCalledWith('bucket', 'folder/key'); }); }); @@ -112,13 +113,17 @@ describe('workflow-helper', () => { describe('createPodLogsMinioRequestConfig', () => { it('returns a MinioRequestConfig factory with the provided minioClientOptions, bucket, and prefix.', async () => { const mockedClient: jest.Mock = MinioClient as any; - const requestFunc = await createPodLogsMinioRequestConfig(minioConfig, 'bucket', 'prefix'); - const request = await requestFunc('workflow-name-abc', 'namespace'); + const requestFunc = await createPodLogsMinioRequestConfig( + minioConfig, + 'bucket', + 'artifacts/{{workflow.name}}/{{workflow.creationTimestamp.Y}}/{{workflow.creationTimestamp.m}}/{{workflow.creationTimestamp.d}}/{{pod.name}}' + ); + const request = await requestFunc('workflow-name-system-container-impl-foo', '2024-08-13', 'namespace'); expect(mockedClient).toBeCalledWith(minioConfig); expect(request.client).toBeInstanceOf(MinioClient); expect(request.bucket).toBe('bucket'); - expect(request.key).toBe('prefix/workflow-name/workflow-name-abc/main.log'); + expect(request.key).toBe('artifacts/workflow-name/2024/08/13/workflow-name-system-container-impl-foo/main.log'); }); }); diff --git a/frontend/server/workflow-helper.ts b/frontend/server/workflow-helper.ts index eb39f7ee9a16..75416000eb03 100644 --- a/frontend/server/workflow-helper.ts +++ b/frontend/server/workflow-helper.ts @@ -128,7 +128,7 @@ export function toGetPodLogsStream( * client). * @param minioOptions Minio options to create a minio client. * @param bucket bucket containing the pod logs artifacts. - * @param prefix prefix for pod logs artifacts stored in the bucket. + * @param keyFormat the keyFormat for pod logs artifacts stored in the bucket. */ export function createPodLogsMinioRequestConfig( minioOptions: MinioClientOptions, diff --git a/frontend/src/lib/Apis.test.ts b/frontend/src/lib/Apis.test.ts index 8c4c2184c529..bd22879fab7f 100644 --- a/frontend/src/lib/Apis.test.ts +++ b/frontend/src/lib/Apis.test.ts @@ -60,7 +60,7 @@ describe('Apis', () => { it('getPodLogs', async () => { const spy = fetchSpy('http://some/address'); - expect(await Apis.getPodLogs('a-run-id', 'some-pod-name', 'ns')).toEqual('http://some/address'); + expect(await Apis.getPodLogs('a-run-id', 'some-pod-name', 'ns', '')).toEqual('http://some/address'); expect(spy).toHaveBeenCalledWith( 'k8s/pod/logs?podname=some-pod-name&runid=a-run-id&podnamespace=ns', { @@ -71,7 +71,7 @@ describe('Apis', () => { it('getPodLogs in a specific namespace', async () => { const spy = fetchSpy('http://some/address'); - expect(await Apis.getPodLogs('a-run-id', 'some-pod-name', 'some-namespace-name')).toEqual( + expect(await Apis.getPodLogs('a-run-id', 'some-pod-name', 'some-namespace-name', '')).toEqual( 'http://some/address', ); expect(spy).toHaveBeenCalledWith( @@ -82,6 +82,19 @@ describe('Apis', () => { ); }); + it('getPodLogs with createdat specified', async () => { + const spy = fetchSpy('http://some/address'); + expect(await Apis.getPodLogs('a-run-id', 'some-pod-name', 'ns', '2024-08-13')).toEqual( + 'http://some/address', + ); + expect(spy).toHaveBeenCalledWith( + 'k8s/pod/logs?podname=some-pod-name&runid=a-run-id&podnamespace=ns&createdat=2024-08-13', + { + credentials: 'same-origin', + }, + ); + }); + it('getPodLogs error', async () => { jest.spyOn(console, 'error').mockImplementation(() => null); window.fetch = jest.fn(() => diff --git a/frontend/src/lib/Apis.ts b/frontend/src/lib/Apis.ts index 366f0dc5e716..ed332ffb25ec 100644 --- a/frontend/src/lib/Apis.ts +++ b/frontend/src/lib/Apis.ts @@ -115,7 +115,9 @@ export class Apis { if (podNamespace) { query += `&podnamespace=${encodeURIComponent(podNamespace)}`; } - query += `&createdat=${encodeURIComponent(createdAt)}`; + if (createdAt) { + query += `&createdat=${encodeURIComponent(createdAt)}`; + } return this._fetch(query); } @@ -429,7 +431,7 @@ export class Apis { '/pipelines/upload_version', v1beta1Prefix, `name=${encodeURIComponent(versionName)}&pipelineid=${encodeURIComponent(pipelineId)}` + - (description ? `&description=${encodeURIComponent(description)}` : ''), + (description ? `&description=${encodeURIComponent(description)}` : ''), { body: fd, cache: 'no-cache', @@ -473,7 +475,7 @@ export class Apis { '/pipelines/upload_version', v2beta1Prefix, `name=${encodeURIComponent(versionName)}&pipelineid=${encodeURIComponent(pipelineId)}` + - (description ? `&description=${encodeURIComponent(description)}` : ''), + (description ? `&description=${encodeURIComponent(description)}` : ''), { body: fd, cache: 'no-cache', @@ -521,7 +523,7 @@ export class Apis { } catch (err) { throw new Error( `Error parsing response for path: ${path}\n\n` + - `Response was: ${responseText}\n\nError was: ${JSON.stringify(err)}`, + `Response was: ${responseText}\n\nError was: ${JSON.stringify(err)}`, ); } }