refactor: DRY test setup and remove inline comments

- Created shared test-helpers.ts with reusable mock utilities
- Refactored UploadStateMachine tests to use shared helpers
- Removed all inline comments from implementation
- Removed console.log debug statements

Reduces test file from 237 to 178 lines (-59 lines)
Reduces implementation from 203 to 164 lines (-39 lines)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Author: abueide

packages/core/src/backoff/UploadStateMachine.ts

@@ -20,15 +20,9 @@ export class UploadStateMachine {
     config: RateLimitConfig,
     logger?: LoggerType
   ) {
-    console.log('[UploadStateMachine] constructor called', {
-      storeId,
-      hasPersistor: !!persistor,
-    });
     this.config = config;
     this.logger = logger;
 
-    // If persistor is provided, try persistent store; fall back to in-memory on error
-    console.log('[UploadStateMachine] About to call createStore...');
     try {
       this.store = createStore<UploadStateData>(
         INITIAL_STATE,
@@ -41,33 +35,18 @@ export class UploadStateMachine {
             }
           : undefined
       );
-      console.log(
-        '[UploadStateMachine] createStore succeeded with persistence'
-      );
       this.logger?.info('[UploadStateMachine] Store created with persistence');
     } catch (e) {
-      console.error(
-        '[UploadStateMachine] createStore with persistence FAILED, falling back to in-memory:',
-        e
-      );
       this.logger?.error(
         `[UploadStateMachine] Persistence failed, using in-memory store: ${e}`
       );
 
-      // Fall back to in-memory store (no persistence)
       try {
         this.store = createStore<UploadStateData>(INITIAL_STATE);
-        console.log(
-          '[UploadStateMachine] Fallback in-memory createStore succeeded'
-        );
         this.logger?.warn(
           '[UploadStateMachine] Using in-memory store (no persistence)'
         );
       } catch (fallbackError) {
-        console.error(
-          '[UploadStateMachine] Even fallback createStore FAILED:',
-          fallbackError
-        );
         this.logger?.error(
           `[UploadStateMachine] CRITICAL: In-memory store creation failed: ${fallbackError}`
         );
@@ -76,14 +55,10 @@ export class UploadStateMachine {
     }
   }
 
-  /**
-   * Upload gate: checks if uploads are allowed
-   * Returns true if READY or if waitUntilTime has passed
-   */
   async canUpload(): Promise<boolean> {
     if (!this.config.enabled) {
       this.logger?.info('[canUpload] Rate limiting disabled, allowing upload');
-      return true; // Legacy behavior when disabled
+      return true;
     }
 
     const state = await this.store.getState();
@@ -98,7 +73,6 @@ export class UploadStateMachine {
       return true;
     }
 
-    // Check if wait period has elapsed
     if (now >= state.waitUntilTime) {
       this.logger?.info(
         '[canUpload] Wait period elapsed, transitioning to READY'
@@ -114,13 +88,10 @@ export class UploadStateMachine {
     return false;
   }
 
-  /**
-   * Handles 429 rate limiting response
-   */
   async handle429(retryAfterSeconds: number): Promise<void> {
     if (!this.config.enabled) {
       this.logger?.info('[handle429] Rate limiting disabled, skipping');
-      return; // No-op when disabled
+      return;
     }
 
     const now = Date.now();
@@ -134,7 +105,6 @@ export class UploadStateMachine {
     const firstFailureTime = state.firstFailureTime ?? now;
     const totalBackoffDuration = (now - firstFailureTime) / 1000;
 
-    // Check max retry count
     if (newRetryCount > this.config.maxRetryCount) {
       this.logger?.warn(
         `Max retry count exceeded (${this.config.maxRetryCount}), resetting rate limiter`
@@ -143,7 +113,6 @@ export class UploadStateMachine {
       return;
     }
 
-    // Check max total backoff duration
     if (totalBackoffDuration > this.config.maxRateLimitDuration) {
       this.logger?.warn(
         `Max backoff duration exceeded (${this.config.maxRateLimitDuration}s), resetting rate limiter`
@@ -165,7 +134,6 @@ export class UploadStateMachine {
       firstFailureTime,
     }));
 
-    // Verify state was set
     const newState = await this.store.getState();
     this.logger?.info(
       `[handle429] AFTER: state=${newState.state}, waitUntil=${newState.waitUntilTime}, globalRetry=${newState.globalRetryCount}`
@@ -176,17 +144,11 @@ export class UploadStateMachine {
     );
   }
 
-  /**
-   * Resets state to READY on successful upload
-   */
   async reset(): Promise<void> {
     await this.store.dispatch(() => INITIAL_STATE);
     this.logger?.info('Upload state reset to READY');
   }
 
-  /**
-   * Gets current global retry count
-   */
   async getGlobalRetryCount(): Promise<number> {
     const state = await this.store.getState();
     return state.globalRetryCount;

packages/core/src/backoff/__tests__/UploadStateMachine.test.ts

@@ -2,47 +2,19 @@ import { UploadStateMachine } from '../UploadStateMachine';
 import type { Persistor } from '@segment/sovran-react-native';
 import type { RateLimitConfig } from '../../types';
 import { getMockLogger } from '../../test-helpers';
+import { createMockStore, createTestPersistor } from './test-helpers';
 
-// Mock sovran-react-native
-jest.mock('@segment/sovran-react-native', () => {
-  const actualModule = jest.requireActual('@segment/sovran-react-native');
-  return {
-    ...actualModule,
-    createStore: jest.fn((initialState: unknown) => {
-      let state = initialState;
-      return {
-        getState: jest.fn(() => Promise.resolve(state)),
-        dispatch: jest.fn((action: unknown) => {
-          // Handle functional dispatch
-          if (typeof action === 'function') {
-            state = action(state);
-          } else {
-            // Handle action object dispatch - add type guard for payload
-            const typedAction = action as { type: string; payload: unknown };
-            state = typedAction.payload;
-          }
-          return Promise.resolve();
-        }),
-      };
-    }),
-  };
-});
+jest.mock('@segment/sovran-react-native', () => ({
+  ...jest.requireActual('@segment/sovran-react-native'),
+  createStore: jest.fn((initialState: unknown) =>
+    createMockStore(initialState)
+  ),
+}));
 
 describe('UploadStateMachine', () => {
-  // Shared storage for all persistors to simulate real persistence
-  let sharedStorage: Record<string, unknown> = {};
-
-  const createMockPersistor = (): Persistor => {
-    return {
-      get: async <T>(key: string): Promise<T | undefined> => {
-        return Promise.resolve(sharedStorage[key] as T);
-      },
-      set: async <T>(key: string, state: T): Promise<void> => {
-        sharedStorage[key] = state;
-        return Promise.resolve();
-      },
-    };
-  };
+  let sharedStorage: Record<string, unknown>;
+  let mockPersistor: Persistor;
+  let mockLogger: ReturnType<typeof getMockLogger>;
 
   const defaultConfig: RateLimitConfig = {
     enabled: true,
@@ -51,12 +23,9 @@ describe('UploadStateMachine', () => {
     maxRateLimitDuration: 43200,
   };
 
-  let mockPersistor: Persistor;
-  let mockLogger: ReturnType<typeof getMockLogger>;
-
   beforeEach(() => {
-    sharedStorage = {}; // Reset shared storage
-    mockPersistor = createMockPersistor();
+    sharedStorage = {};
+    mockPersistor = createTestPersistor(sharedStorage);
     mockLogger = getMockLogger();
     jest.clearAllMocks();
   });
@@ -70,48 +39,40 @@ describe('UploadStateMachine', () => {
         mockLogger
       );
 
-      const canUpload = await stateMachine.canUpload();
-      expect(canUpload).toBe(true);
+      expect(await stateMachine.canUpload()).toBe(true);
     });
 
-    it('returns false when in WAITING state and waitUntilTime not passed', async () => {
+    it('returns false when in RATE_LIMITED state and waitUntilTime not passed', async () => {
       const stateMachine = new UploadStateMachine(
         'test-key',
         mockPersistor,
         defaultConfig,
         mockLogger
       );
 
-      // Set to waiting state with future time
       await stateMachine.handle429(60);
 
-      const canUpload = await stateMachine.canUpload();
-      expect(canUpload).toBe(false);
+      expect(await stateMachine.canUpload()).toBe(false);
       expect(mockLogger.info).toHaveBeenCalledWith(
         expect.stringContaining('Upload blocked: rate limited')
       );
     });
 
     it('transitions to READY and returns true when waitUntilTime has passed', async () => {
+      const now = 1000000;
+      jest.spyOn(Date, 'now').mockReturnValue(now);
+
       const stateMachine = new UploadStateMachine(
         'test-key',
         mockPersistor,
         defaultConfig,
         mockLogger
       );
 
-      // Mock Date.now to control time
-      const now = 1000000;
-      jest.spyOn(Date, 'now').mockReturnValue(now);
-
-      // Set to waiting state
       await stateMachine.handle429(60);
-
-      // Advance time past waitUntilTime
       jest.spyOn(Date, 'now').mockReturnValue(now + 61000);
 
-      const canUpload = await stateMachine.canUpload();
-      expect(canUpload).toBe(true);
+      expect(await stateMachine.canUpload()).toBe(true);
       expect(mockLogger.info).toHaveBeenCalledWith(
         'Upload state transitioned to READY'
       );
@@ -131,17 +92,10 @@ describe('UploadStateMachine', () => {
       );
 
       await stateMachine.handle429(60);
-
-      // Wait for store update
       await new Promise((resolve) => setTimeout(resolve, 50));
 
-      const globalRetryCount = await stateMachine.getGlobalRetryCount();
-      expect(globalRetryCount).toBe(1);
-
-      // Verify it's in WAITING state
-      const canUpload = await stateMachine.canUpload();
-      expect(canUpload).toBe(false);
-
+      expect(await stateMachine.getGlobalRetryCount()).toBe(1);
+      expect(await stateMachine.canUpload()).toBe(false);
       expect(mockLogger.info).toHaveBeenCalledWith(
         'Rate limited (429): waiting 60s before retry 1/100'
       );
@@ -165,17 +119,13 @@ describe('UploadStateMachine', () => {
       await new Promise((resolve) => setTimeout(resolve, 50));
       await stateMachine.handle429(10);
       await new Promise((resolve) => setTimeout(resolve, 50));
-
-      // 4th attempt should reset
       await stateMachine.handle429(10);
       await new Promise((resolve) => setTimeout(resolve, 50));
 
       expect(mockLogger.warn).toHaveBeenCalledWith(
         'Max retry count exceeded (3), resetting rate limiter'
       );
-
-      const retryCount = await stateMachine.getGlobalRetryCount();
-      expect(retryCount).toBe(0); // Reset to 0
+      expect(await stateMachine.getGlobalRetryCount()).toBe(0);
     });
 
     it('resets state when max total backoff duration exceeded', async () => {
@@ -184,7 +134,7 @@ describe('UploadStateMachine', () => {
 
       const limitedConfig: RateLimitConfig = {
         ...defaultConfig,
-        maxRateLimitDuration: 10, // Only 10 seconds allowed
+        maxRateLimitDuration: 10,
       };
       const stateMachine = new UploadStateMachine(
         'test-key',
@@ -194,18 +144,13 @@ describe('UploadStateMachine', () => {
       );
 
       await stateMachine.handle429(5);
-
-      // Advance time beyond maxRateLimitDuration
       jest.spyOn(Date, 'now').mockReturnValue(now + 11000);
-
       await stateMachine.handle429(5);
 
       expect(mockLogger.warn).toHaveBeenCalledWith(
         'Max backoff duration exceeded (10s), resetting rate limiter'
       );
-
-      const retryCount = await stateMachine.getGlobalRetryCount();
-      expect(retryCount).toBe(0);
+      expect(await stateMachine.getGlobalRetryCount()).toBe(0);
     });
   });
 
@@ -218,12 +163,10 @@ describe('UploadStateMachine', () => {
         mockLogger
       );
 
-      // Put into WAITING state
       await stateMachine.handle429(60);
       await new Promise((resolve) => setTimeout(resolve, 50));
       expect(await stateMachine.getGlobalRetryCount()).toBe(1);
 
-      // Reset
       await stateMachine.reset();
       await new Promise((resolve) => setTimeout(resolve, 50));
 

packages/core/src/backoff/__tests__/test-helpers.ts

@@ -0,0 +1,27 @@
+import type { Persistor } from '@segment/sovran-react-native';
+
+export const createMockStore = <T>(initialState: T) => {
+  let state = initialState;
+  return {
+    getState: jest.fn(() => Promise.resolve(state)),
+    dispatch: jest.fn((action: unknown) => {
+      if (typeof action === 'function') {
+        state = action(state);
+      } else {
+        state = (action as { payload: unknown }).payload as T;
+      }
+      return Promise.resolve();
+    }),
+  };
+};
+
+export const createTestPersistor = (
+  storage: Record<string, unknown> = {}
+): Persistor => ({
+  get: async <T>(key: string): Promise<T | undefined> =>
+    Promise.resolve(storage[key] as T),
+  set: async <T>(key: string, state: T): Promise<void> => {
+    storage[key] = state;
+    return Promise.resolve();
+  },
+});