test(app): pin the lock-error sink of redisStores, the deadline cap of redisLocks and the hundred-per-page lister

__tests__/adapters/app/cron.test.ts

@@ -188,9 +188,11 @@ describe('appInstallationLister', () => {
   const api = 'https://ghes.example/api/v3'
   const { privateKey } = generateKeyPairSync('rsa', { modulusLength: 2048, privateKeyEncoding: { type: 'pkcs1', format: 'pem' }, publicKeyEncoding: { type: 'pkcs1', format: 'pem' } })
 
-  it('lists every repository of every installation over every page, skipping suspended installations and archived repositories', async () => {
+  it('lists every repository of every installation over every page, a hundred at a time, skipping suspended installations and archived repositories', async () => {
+    const pageSizes: string[] = []
     server.use(
       http.get(`${api}/app/installations`, ({ request }) => {
+        pageSizes.push(`installations ${new URL(request.url).searchParams.get('per_page')}`)
         const p = new URL(request.url).searchParams.get('page') ?? '1'
         const headers: Record<string, string> = { 'Content-Type': 'application/json' }
         if (p === '1') {
@@ -203,6 +205,7 @@ describe('appInstallationLister', () => {
       }),
       http.post(`${api}/app/installations/:id/access_tokens`, ({ params }) => new Response(JSON.stringify({ token: `tok-${params.id}`, expires_at: new Date(Date.now() + 3_600_000).toISOString(), permissions: {}, repository_selection: 'all' }), { status: 201, headers: { 'Content-Type': 'application/json' } })),
       http.get(`${api}/installation/repositories`, ({ request }) => {
+        pageSizes.push(`repositories ${new URL(request.url).searchParams.get('per_page')}`)
         const token = request.headers.get('authorization')
         const repos = token === 'token tok-1'
           ? [{ name: 'api', full_name: 'acme/api', owner: { login: 'acme' }, archived: false }, { name: 'old', full_name: 'acme/old', owner: { login: 'acme' }, archived: true }]
@@ -217,6 +220,8 @@ describe('appInstallationLister', () => {
     }
 
     expect(out.map(t => `${t.installationId}:${t.repository.full_name}`)).toEqual(['1:acme/api', '2:globex/core'])
+    // the largest page GitHub serves: an App on many installations or repositories lists them in the fewest requests
+    expect(pageSizes).toEqual(['installations 100', 'installations 100', 'repositories 100', 'repositories 100'])
   })
 
   it.each([

__tests__/adapters/app/redis.test.ts

@@ -125,7 +125,7 @@ describe('redisLocks', () => {
     expect(sleep.mock.calls).toEqual([[600], [1000], [1000]])
   })
 
-  it('waits five minutes for a held lock by default before giving up', async () => {
+  it('waits five minutes for a held lock by default before giving up, and never sleeps past that deadline', async () => {
     const client = fakeRedis()
     let now = 1_000_000
     vi.spyOn(Date, 'now').mockImplementation(() => now)
@@ -136,7 +136,8 @@ describe('redisLocks', () => {
 
     await expect(redisLocks(client).withLock('k', async () => {})).rejects.toThrow('could not lock k within 300000ms')
     expect(lockWaitMs).toBe(300_000)
-    expect(now).toBeGreaterThanOrEqual(1_000_000 + lockWaitMs)
+    // the last wait is cut to what is left of the deadline, so the wait ends on it, not up to a second after
+    expect(now).toBe(1_000_000 + lockWaitMs)
   })
 
   it('renews the lock every third of the ttl while fn runs, and stops once it returns', async () => {

__tests__/server/stores.test.ts

@@ -67,6 +67,21 @@ describe('storesFromEnv', () => {
     await stores.close()
     expect(client.close).toHaveBeenCalled()
   })
+
+  it('hands the sink to the redis locks, so a release that fails is logged rather than lost', async () => {
+    const lines: Record<string, unknown>[] = []
+    const stores = storesFromEnv({ REDIS_URL: 'redis://x' }, line => lines.push(line))
+    await stores.ready
+    const { createClient } = await import('redis')
+    const client = vi.mocked(createClient).mock.results.at(-1)!.value as { eval: () => Promise<unknown> }
+    client.eval = async () => {
+      throw new Error('offline')
+    }
+
+    await expect(stores.locks.withLock('o/r#1', async () => 'handled')).resolves.toBe('handled')
+
+    expect(lines).toContainEqual({ level: 'error', message: 'redis lock o/r#1: release failed: offline' })
+  })
 })
 
 describe('the server with stores', () => {