From ca6618a1c4076e24a1dcf99acae54f150df784fe Mon Sep 17 00:00:00 2001
From: John Kaster <kaster@google.com>
Date: Wed, 30 Jun 2021 16:55:46 +0000
Subject: [PATCH] PR feedback for unit test clarifications

Also added functional test to verify pageAll works with non-paging endpoints
---
 packages/sdk-node/test/methods.spec.ts | 26 ++++++++++++++
 packages/sdk-rtl/src/paging.spec.ts    | 48 ++++++++++++++++----------
 2 files changed, 55 insertions(+), 19 deletions(-)

diff --git a/packages/sdk-node/test/methods.spec.ts b/packages/sdk-node/test/methods.spec.ts
index 429eaa59d..2495bfeef 100644
--- a/packages/sdk-node/test/methods.spec.ts
+++ b/packages/sdk-node/test/methods.spec.ts
@@ -692,6 +692,32 @@ describe('LookerNodeSDK', () => {
         },
         testTimeout
       )
+      test(
+        'all_dashboards pageAll returns non-paged results',
+        async () => {
+          const sdk = new LookerSDK(session)
+          // Use a small limit to test paging for a small number of dashboards
+          let count = 0
+          let actual: IDashboard[] = []
+          const aggregate = (page: IDashboard[]) => {
+            console.log(`Page ${++count} has ${page.length} items`)
+            actual = actual.concat(page)
+            return page
+          }
+          const paged = await pageAll(
+            sdk,
+            () => sdk.all_dashboards('id,title'),
+            aggregate
+          )
+          expect(paged.limit).toEqual(-1)
+          expect(paged.more()).toEqual(false)
+
+          const all = await sdk.ok(sdk.all_dashboards('id, title'))
+          expect(actual.length).toEqual(all.length)
+          expect(actual).toEqual(all)
+        },
+        testTimeout
+      )
     })
   })
 
diff --git a/packages/sdk-rtl/src/paging.spec.ts b/packages/sdk-rtl/src/paging.spec.ts
index 71e174d29..36e054bec 100644
--- a/packages/sdk-rtl/src/paging.spec.ts
+++ b/packages/sdk-rtl/src/paging.spec.ts
@@ -47,6 +47,9 @@ const nextUrl =
   'http://localhost/api/4.0/alerts/search?fields=id&limit=3&offset=6'
 
 const firstLink = `<${firstUrl}>; rel="first"`
+// Verify extra whitespace characters still get parsed correctly
+// The Looker API provides a header without extra whitespace but other Link headers
+// may not be formatted that way
 const lastLink = `< ${lastUrl} >; rel="\tlast (end of line!)\t"`
 const prevLink = `<\t${prevUrl}\n>;\nrel="prev"`
 const nextLink = `<${nextUrl}>; rel="next"`
@@ -81,25 +84,31 @@ const session = new MockSession('mocked', settings, transport)
 const sdk = new APIMethods(session, '4.0')
 const mockedRows = ['one', 'two', 'three', 'four', 'five']
 const totalCount = 10
-const mockedRawResponse: IRawResponse = {
-  ok: true,
-  body: JSON.stringify(mockedRows),
-  headers: { [LinkHeader]: allLinks, [TotalCountHeader]: ` ${totalCount}` },
-  statusCode: 200,
-  statusMessage: 'Mocking',
-  contentType: 'application/json',
-  url: 'https://mocked',
-}
 
-const mockRawResponse = (url: string, body?: any): IRawResponse => {
-  const result = { ...mockedRawResponse, ...{ url: url } }
+const mockRawResponse = (url?: string, body?: any): IRawResponse => {
+  const result: IRawResponse = {
+    ok: true,
+    body: JSON.stringify(mockedRows),
+    headers: { [LinkHeader]: allLinks, [TotalCountHeader]: ` ${totalCount}` },
+    statusCode: 200,
+    statusMessage: 'Mocking',
+    contentType: 'application/json',
+    url: 'https://mocked',
+  }
+  if (url) {
+    result.url = url
+  }
   if (body) {
     result.body = body
   }
   return result
 }
 
-const mockRaw = (
+async function mockSDKSuccess<T>(value: T) {
+  return Promise.resolve<SDKResponse<T, any>>({ ok: true, value })
+}
+
+const mockRawResponseSuccess = (
   transport: BrowserTransport,
   value: any,
   rawResponse: IRawResponse
@@ -110,10 +119,6 @@ const mockRaw = (
   return mockSDKSuccess(value)
 }
 
-async function mockSDKSuccess<T>(value: T) {
-  return Promise.resolve<SDKResponse<T, any>>({ ok: true, value })
-}
-
 // async function mockSDKError<T>(value: T) {
 //   return Promise.resolve<SDKResponse<any, T>>({ ok: false, error: value })
 // }
@@ -158,7 +163,7 @@ describe('paging', () => {
     beforeEach(() => {
       jest
         .spyOn(BrowserTransport.prototype, 'rawRequest')
-        .mockReturnValue(Promise.resolve(mockedRawResponse))
+        .mockReturnValue(Promise.resolve(mockRawResponse()))
     })
     afterAll(() => {
       jest.clearAllMocks()
@@ -167,7 +172,12 @@ describe('paging', () => {
     it('initializes', async () => {
       const actual = await pager(
         sdk,
-        () => mockRaw(transport, mockedRows, mockRawResponse(firstUrl)),
+        () =>
+          mockRawResponseSuccess(
+            transport,
+            mockedRows,
+            mockRawResponse(firstUrl)
+          ),
         {
           timeout: 99,
         }
@@ -189,7 +199,7 @@ describe('paging', () => {
 
     it('supports paging', async () => {
       const paged = await pager(sdk, () =>
-        mockRaw(transport, mockedRows, mockedRawResponse)
+        mockRawResponseSuccess(transport, mockedRows, mockRawResponse())
       )
       expect(paged).toBeDefined()
       expect(paged.items).toEqual(mockedRows)