Update frontend unit tests

Signed-off-by: droctothorpe <mythicalsunlight@gmail.com>
kubeflow · Aug 13, 2024 · 66b76a0 · 66b76a0
1 parent dd7692a
commit 66b76a0
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 23 deletions.
diff --git 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,24 +102,28 @@ 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');
     });
   });
 
   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
@@ -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
@@ -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
@@ -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)}`,
       );
     }
   }