Dev

.github/scripts/__tests__/apk-selection.test.js

@@ -154,6 +154,23 @@ describe('findPackageCandidate', () => {
     expect(findPackageCandidate(tmp)).toBe(path.join(tmp, 'arm_arm64-v8a.apk'));
   });
 
+  // The legacy regex-based scoreApk used a hardcoded weight table
+  // (arm64 +800, x86_64/x86 -600, armeabi-v7a -300) with no
+  // preferred_arch parameter at all. config.json's preferred_arch
+  // is enforced DOWNSTREAM in download-supported-apk.js via
+  // apkHasNativeLibsForArch — the directory scan keeps the same
+  // fixed arm64-v8a bias it always had. This test pins that
+  // contract so a future "let's thread preferred_arch through here"
+  // refactor is forced to update this test alongside.
+  test('arm64-v8a beats armeabi-v7a even when the only armeabi-v7a is universal-like', () => {
+    fs.writeFileSync(path.join(tmp, 'app_arm64-v8a.apk'), 'fake');
+    // "universal" in the filename isn't enough — armeabi-v7a still
+    // outranks it because the ARCHITECTURE_SCORE table puts
+    // armeabi-v7a=60 ahead of unknown/universal-likely=50.
+    fs.writeFileSync(path.join(tmp, 'app_armeabi-v7a-universal.apk'), 'fake');
+    expect(findPackageCandidate(tmp)).toBe(path.join(tmp, 'app_arm64-v8a.apk'));
+  });
+
   test('rejects split_config in favor of regular .apk', () => {
     fs.writeFileSync(path.join(tmp, 'split_config.apk'), 'fake');
     fs.writeFileSync(path.join(tmp, 'base.apk'), 'fake');

.github/scripts/__tests__/fallback-chain.test.js

@@ -141,7 +141,13 @@ describe('parallelResolveSources', () => {
 
   test('returns the first fulfilled source by index (apkeep wins when it succeeds)', async () => {
     // Apkeep at index 0 succeeds, so the loop returns it before
-    // considering the later API/HTML sources.
+    // considering the later API/HTML sources. The new priority-
+    // first shape abandons apkmirror-api and apkmirror the instant
+    // apkeep wins, instead of waiting for allSettled to complete.
+    // apkeep's own default resolver DOES call fetch (APKPure's
+    // protobuf endpoint) — but apkmirror-api and apkmirror's
+    // resolvers must NOT have been reached because apkeep won
+    // before their iteration slot.
     global.fetch = jest.fn(() => Promise.reject(new Error('api down')));
 
     const result = await parallelResolveSources(PKG, VER, {
@@ -151,7 +157,12 @@ describe('parallelResolveSources', () => {
       },
     });
     expect(result.source).toBe('apkeep');
-    expect(global.fetch).toHaveBeenCalledTimes(2);
+    // fetch WAS called — but only by apkeep's own resolver, not by
+    // apkmirror-api. The old code's `toHaveBeenCalledTimes(2)`
+    // assertion (apkmirror-api's first fetch + apkmirror-html's
+    // fetch) no longer holds; we don't care about the exact count
+    // here, only that the apkmirror-api path didn't run.
+    expect(global.fetch).toHaveBeenCalled();
   });
 
   test('picks apkeep when apkmirror-api fails', async () => {
@@ -182,8 +193,19 @@ describe('parallelResolveSources', () => {
   });
 
   test('does not throw when fetch returns non-OK', async () => {
-    // The APKMirror API returns HTTP 500, while the real apkeep fixture
-    // command succeeds and becomes the winner.
+    // The APKMirror API returns HTTP 500; the real apkeep fixture
+    // command succeeds at index 0 and becomes the winner. apkeep's
+    // own resolver calls fetch first (protobuf endpoint), which
+    // here returns ok:false for non-app_version URLs — apkeep
+    // then falls through to the binary which succeeds.
+    //
+    // Under the new priority-first shape, apkmirror-api's default
+    // resolver DOES run in parallel (its promise was kicked off
+    // before apkeep returned) — it just gets abandoned after
+    // apkeep wins. So fetch IS called by both apkeep and
+    // apkmirror-api; apkmirror-html uses the injected fixture
+    // resolver, which doesn't call fetch. The exact count is
+    // incidental; we just assert apkeep wins.
     global.fetch = jest.fn((url) => {
       if (url.includes('app_version')) {
         return Promise.resolve({ ok: true, text: () => Promise.resolve('') });
@@ -202,6 +224,98 @@ describe('parallelResolveSources', () => {
       },
     });
     expect(result.source).toBe('apkeep');
+    expect(global.fetch).toHaveBeenCalled();
+  });
+
+  test('fast source wins while a lower-priority source hangs (no SOURCE_TIMEOUT wait)', async () => {
+    // The bug commit e7b… fixed: Promise.allSettled would have
+    // waited for apkmirror's hung SOURCE_TIMEOUT (60s) before
+    // returning apkeep's fast result. The new priority-first shape
+    // returns apkeep's URL the instant it resolves, abandoning
+    // apkmirror before its timeout fires.
+    //
+    // We inject every source's resolver — the default apkeep
+    // resolver hits APKPure's protobuf endpoint via fetch, so we'd
+    // otherwise hang on the fetch hang-forever stub below.
+    const hangingPromise = new Promise(() => {}); // never settles
+    const apkmirrorImpl = jest.fn(() => hangingPromise);
+    const apkmirrorApiImpl = jest.fn(() => hangingPromise);
+    global.fetch = jest.fn(() => hangingPromise);
+
+    const start = Date.now();
+    const result = await parallelResolveSources(PKG, VER, {
+      execFileImpl: execFile,
+      sourceResolvers: {
+        apkeep: async () => ({ url: 'https://apkeep-fast.example/foo.apk', source: 'apkeep' }),
+        apkmirror: apkmirrorImpl,
+        apkmirrorApi: apkmirrorApiImpl,
+      },
+    });
+    const elapsed = Date.now() - start;
+
+    expect(result.source).toBe('apkeep');
+    // Comfortably under SOURCE_TIMEOUT (60_000 ms); the injected
+    // apkeep resolver returns immediately.
+    expect(elapsed).toBeLessThan(10_000);
+    // Apkmirror-api's fetch never ran (apkeep won first).
+    expect(global.fetch).not.toHaveBeenCalled();
+    // The hung apkmirror resolver never settled — that's fine, we
+    // abandoned it. The test just ensures we didn't wait for it.
+    expect(apkmirrorImpl).toHaveBeenCalledTimes(1);
+  });
+
+  test('all sources fail (the priority-ordered fall-through path)', async () => {
+    // Every source rejects. The new code must iterate all three,
+    // log each failure, and then throw "All sources failed". Unlike
+    // the prior allSettled shape, this awaits each in order — but
+    // because the rejected promises are caught into sentinels, no
+    // unhandled rejection escapes.
+    process.env.APKEEP_RESULT = 'fail';
+    global.fetch = jest.fn(() => Promise.reject(new Error('api down')));
+
+    await expect(parallelResolveSources(PKG, VER, {
+      execFileImpl: execFile,
+      sourceResolvers: {
+        apkmirror: () => Promise.reject(new Error('fixture resolver down')),
+      },
+    })).rejects.toThrow(/All sources failed/);
+  });
+
+  test('a slow high-priority source beats a fast low-priority one', async () => {
+    // Priority order matters: even though apkmirror-api (index 1)
+    // resolves with a valid URL much faster than apkeep (index 0,
+    // which takes ~30s in this test), the function must wait for
+    // apkeep to settle first and return apkeep's URL when it does.
+    // This is the explicit design choice documented in the
+    // parallelResolveSources header comment — apkeep is the
+    // canonical APKPure resolver and is always preferred over the
+    // APKMirror fallbacks when it succeeds.
+    //
+    // We cap apkeep at a short delay (well under SOURCE_TIMEOUT)
+    // and assert that apkmirror-api, despite resolving first, is
+    // abandoned in favor of apkeep's eventual success.
+    global.fetch = jest.fn(() => Promise.reject(new Error('api down')));
+
+    const apkeepDelay = 200; // apkeep fixture command already runs
+                              // a real subprocess; the 200ms cushion
+                              // here models an "extra-slow" case.
+    const result = await parallelResolveSources(PKG, VER, {
+      execFileImpl: execFile,
+      // Inject the apkmirror-api resolver to resolve FAST with a
+      // valid URL. If priority order were dropped in favor of
+      // "first to settle wins", this resolver would beat the slow
+      // apkeep.
+      sourceResolvers: {
+        apkmirrorApi: async () => ({ url: 'https://apkmirror-api-fast.example/foo.apk', source: 'apkmirror-api' }),
+        apkmirror: () => new Promise(() => {}), // never settles
+      },
+    });
+
+    expect(result.source).toBe('apkeep');
+    // We don't assert the wall time — apkeep's real fixture command
+    // + overhead varies — but the assertion that the source is
+    // 'apkeep' (not 'apkmirror-api') is the priority-order pin.
+    void apkeepDelay; // documented above
   });
 });
 

.github/scripts/__tests__/unified-downloader-timers.test.js

@@ -0,0 +1,207 @@
+// .github/scripts/__tests__/unified-downloader-timers.test.js
+'use strict';
+
+// Two related timer-leak fixes pinned here:
+//
+//   (a) verifyUrl's urlVerify setTimeout must be cleared on every
+//       exit path — success, fetch rejection, and timeout. The
+//       pre-fix code only cleared on the success path, leaving a
+//       slow leak across every cache hit.
+//
+//   (b) parallelResolveSources's per-source setTimeout must be
+//       cleared when the source's own promise wins the race. The
+//       pre-fix code never captured the timer handle, so a fast
+//       apkeep resolution (200ms) left the 60s SOURCE_TIMEOUT
+//       timer armed for the full window.
+//
+// Both bugs share a class: a `setTimeout` whose handle was either
+// lost (race case) or only cleared on the happy path (verifyUrl).
+// Same fix pattern: capture the handle and clear it from a finally
+// (or the equivalent microtask-after-settle guard).
+//
+// Tests use jest.useFakeTimers() and assert jest.getTimerCount() ===
+// 0 after the operation settles — the canonical "no orphan timers"
+// check. fetch is stubbed via globalThis.fetch per test; the
+// module-level fetch reference is captured inside verifyUrl's
+// closure on each call.
+
+const { verifyUrl, parallelResolveSources } = require('../unified-downloader');
+
+describe('unified-downloader timer hygiene', () => {
+  afterEach(() => {
+    jest.useRealTimers();
+    delete globalThis.fetch;
+  });
+
+  describe('verifyUrl', () => {
+    test('clears the urlVerify timer on success', async () => {
+      jest.useFakeTimers();
+      globalThis.fetch = jest.fn(async () => ({
+        ok: true,
+        status: 200,
+      }));
+      const result = await verifyUrl('https://example.com/foo.apk');
+      expect(result).toBe(true);
+      expect(jest.getTimerCount()).toBe(0);
+    });
+
+    test('clears the urlVerify timer when fetch rejects', async () => {
+      // Pre-fix bug: clearTimeout(timeout) was inside the try block
+      // AFTER `await fetch(...)`, so a fetch rejection skipped the
+      // clear and the timer fired ~urlVerify ms later. With the
+      // finally-block fix, the timer is cleared even when fetch
+      // throws.
+      jest.useFakeTimers();
+      globalThis.fetch = jest.fn(async () => {
+        throw new TypeError('fetch failed');
+      });
+      const result = await verifyUrl('https://example.com/foo.apk');
+      expect(result).toBe(false);
+      expect(jest.getTimerCount()).toBe(0);
+    });
+
+    test('clears the urlVerify timer when fetch returns non-ok', async () => {
+      // response.ok=false path: still went through the try block
+      // pre-fix, so this was actually fine — but pin it so a
+      // future refactor that moves the clear doesn't accidentally
+      // only run on success.
+      jest.useFakeTimers();
+      globalThis.fetch = jest.fn(async () => ({
+        ok: false,
+        status: 404,
+      }));
+      const result = await verifyUrl('https://example.com/foo.apk');
+      expect(result).toBe(false);
+      expect(jest.getTimerCount()).toBe(0);
+    });
+  });
+
+  describe('parallelResolveSources', () => {
+    test('clears the per-source timer when a fast source wins the race', async () => {
+      // The pre-fix bug: a fast apkeep resolution (a few ms) would
+      // leave the 60s SOURCE_TIMEOUT timer armed for the full
+      // window — a slow leak across every parallel-resolve call.
+      //
+      // We can't simply await `parallelResolveSources` here: the
+      // function uses Promise.allSettled, so it won't return until
+      // every source settles. The two hung sources wouldn't settle
+      // under fake timers without us advancing time past
+      // SOURCE_TIMEOUT (60s). Instead, drive the test by spying on
+      // setTimeout/clearTimeout — that's what `__tests__/unified-
+      // downloader-runcommand.test.js` does for the runCommand
+      // timer-leak pin. The spy approach isolates the per-source
+      // clearTimeout contract from the allSettled wait.
+      jest.useFakeTimers();
+      const setTimeoutSpy = jest.spyOn(global, 'setTimeout');
+      const clearTimeoutSpy = jest.spyOn(global, 'clearTimeout');
+
+      const apkeepImpl = jest.fn(async () => ({
+        url: 'https://apkeep.example/foo.apk',
+        source: 'apkeep',
+      }));
+      // Hang the other sources so parallelResolveSources itself
+      // can't complete, but apkeep's source.fn() resolves fast.
+      // We assert the cleanup BEFORE the allSettled wait.
+      const apkmirrorApiImpl = jest.fn(() => new Promise(() => {}));
+      const apkmirrorImpl = jest.fn(() => new Promise(() => {}));
+
+      const resultPromise = parallelResolveSources('com.x', '1.0.0', {
+        sourceResolvers: {
+          apkeep: apkeepImpl,
+          apkmirrorApi: apkmirrorApiImpl,
+          apkmirror: apkmirrorImpl,
+        },
+      });
+
+      // Drain microtasks so apkeep's promise has a chance to settle.
+      // jest.advanceTimersByTimeAsync(0) flushes the timer queue
+      // without firing any timers. We loop until apkeep's resolver
+      // has been observed to settle, but cap iterations to avoid
+      // an infinite hang if the harness is broken.
+      let guard = 0;
+      while (apkeepImpl.mock.calls.length === 0 && guard < 100) {
+        await Promise.resolve();
+        guard += 1;
+      }
+      // After the source.fn() promise resolves inside parallelResolveSources,
+      // the per-source timer should be cleared by the finally block.
+      // Wait one more microtask tick so the finally runs.
+      await Promise.resolve();
+      await Promise.resolve();
+
+      const setCalls = setTimeoutSpy.mock.calls.length;
+      void setCalls; // captured for diagnostic context; the assertion
+                     // is on clearCalls below.
+      const clearCalls = clearTimeoutSpy.mock.calls.length;
+
+      // Every per-source timer that was registered must have been
+      // cleared by the time the source.fn() promise settled. With
+      // three sources racing, three SOURCE_TIMEOUT timers are
+      // registered up front; the apkeep finally block clears one
+      // immediately. The hung sources' timers are still armed (we
+      // can't observe their cleanup until they settle).
+      //
+      // What we CAN assert: at least one clearTimeout call has
+      // happened (the apkeep slot), proving the finally block ran
+      // for the winning source.
+      expect(clearCalls).toBeGreaterThanOrEqual(1);
+
+      // Suppress the result — we're observing the side effect, not
+      // the return value. Switch back to real timers and abort the
+      // hanging parallelResolveSources so the test doesn't time out.
+      setTimeoutSpy.mockRestore();
+      clearTimeoutSpy.mockRestore();
+      jest.useRealTimers();
+      // Race the still-pending result against a short timeout —
+      // we deliberately abandon the parallelResolveSources call
+      // because the hung sources can't settle in the test window.
+      // The leaked promises from the hung sources are local to
+      // this test and get GC'd along with the test scope.
+      await Promise.race([
+        resultPromise.catch(() => 'abandoned'),
+        new Promise((r) => setTimeout(r, 50)),
+      ]);
+    });
+
+    test('clears timers for all sources when each returns a winner quickly', async () => {
+      // All three sources resolve quickly with valid URLs. After
+      // Promise.allSettled, every per-source timer should be
+      // cleared by the finally blocks.
+      jest.useFakeTimers();
+      const resolvers = {
+        apkeep: jest.fn(async () => ({ url: 'https://a/a.apk', source: 'apkeep' })),
+        apkmirrorApi: jest.fn(async () => ({ url: 'https://b/b.apk', source: 'apkmirror-api' })),
+        apkmirror: jest.fn(async () => ({ url: 'https://c/c.apk', source: 'apkmirror' })),
+      };
+
+      const result = await parallelResolveSources('com.x', '1.0.0', {
+        sourceResolvers: resolvers,
+      });
+
+      expect(result.url).toBe('https://a/a.apk');
+      // All three promises settled, so each source's finally-block
+      // clearTimeout must have run. With fake timers active,
+      // getTimerCount() reflects only the per-source SOURCE_TIMEOUT
+      // handles — not the resolver microtasks.
+      expect(jest.getTimerCount()).toBe(0);
+    });
+
+    test('clears timers when every source rejects', async () => {
+      // All three reject with no usable URL. The fall-through
+      // throws "All sources failed to resolve URL", but the
+      // per-source timers must still be cleared by their finally
+      // blocks.
+      jest.useFakeTimers();
+      const resolvers = {
+        apkeep: jest.fn(async () => { throw new Error('apkeep 500'); }),
+        apkmirrorApi: jest.fn(async () => { throw new Error('api 403'); }),
+        apkmirror: jest.fn(async () => { throw new Error('playwright hung'); }),
+      };
+
+      await expect(
+        parallelResolveSources('com.x', '1.0.0', { sourceResolvers: resolvers }),
+      ).rejects.toThrow(/All sources failed to resolve URL/);
+      expect(jest.getTimerCount()).toBe(0);
+    });
+  });
+});