Skip to content

[v8] Improve performance of audit log tests#1408

Merged
nicknisi merged 1 commit intoversion-8from
nicknisi/audit-log-tests
Dec 2, 2025
Merged

[v8] Improve performance of audit log tests#1408
nicknisi merged 1 commit intoversion-8from
nicknisi/audit-log-tests

Conversation

@nicknisi
Copy link
Member

@nicknisi nicknisi commented Dec 2, 2025

Summary

  • Use Jest fake timers instead of real timers in audit log retry tests
  • Tests now run instantly instead of waiting for actual retry delays
  • Removes timeout extensions that were needed for slow tests

@nicknisi nicknisi requested a review from a team as a code owner December 2, 2025 01:37
@nicknisi nicknisi requested review from kendallstrautman and removed request for a team December 2, 2025 01:37
@nicknisi nicknisi changed the title Improve performance of audit log tests [v8] Improve performance of audit log tests Dec 2, 2025
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 2, 2025

Greptile Overview

Greptile Summary

Optimized audit log retry tests by replacing real timers with Jest fake timers. Tests now execute instantly instead of waiting for actual retry delays (previously took 10+ seconds).

Key Changes:

  • Added jest.useFakeTimers() in beforeEach and cleanup in afterEach for the retry behavior test suite
  • Modified test pattern: store promise, call jest.runAllTimersAsync(), then await promise
  • Removed 10-second timeout extensions from slow tests (no longer needed)
  • Fixed error handling test to use .catch() pattern before advancing timers

Impact:

  • Significantly faster test execution (from ~10s to instant)
  • Maintains test correctness - still validates retry logic, status codes, and idempotency
  • Better developer experience with faster feedback loops

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The changes are isolated to test code only, with no production code modified. The test logic remains functionally identical - it still validates the same retry behavior, error handling, and idempotency. The switch to fake timers is a standard Jest testing pattern that improves performance without affecting correctness. All tests properly clean up with useRealTimers() to avoid affecting other tests.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
src/audit-logs/audit-logs.spec.ts 5/5 Improved retry test performance by using Jest fake timers instead of real delays - tests now run instantly while maintaining correctness

Sequence Diagram

sequenceDiagram
    participant Test as Test Suite
    participant Jest as Jest Fake Timers
    participant WorkOS as WorkOS Client
    participant HTTP as HTTP Client
    participant API as Mock API

    Test->>Jest: jest.useFakeTimers()
    Test->>WorkOS: createEvent('org_123', event)
    WorkOS->>HTTP: post('/audit_logs/events')
    HTTP->>API: Attempt 1
    API-->>HTTP: 500 Error
    HTTP->>HTTP: shouldRetryRequest() = true
    HTTP->>Jest: setTimeout(sleep, ~500ms)
    Note over HTTP,Jest: Timer scheduled but not executed
    
    Test->>Jest: runAllTimersAsync()
    Jest->>HTTP: Execute sleep callback
    HTTP->>API: Attempt 2
    API-->>HTTP: 500 Error
    HTTP->>HTTP: shouldRetryRequest() = true
    HTTP->>Jest: setTimeout(sleep, ~750ms)
    Jest->>HTTP: Execute sleep callback
    HTTP->>API: Attempt 3
    API-->>HTTP: 201 Success
    HTTP-->>WorkOS: Success response
    WorkOS-->>Test: Promise resolves
    
    Test->>Jest: jest.useRealTimers()
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

@nicknisi nicknisi force-pushed the nicknisi/audit-log-tests branch from 3eb5cff to 1b5124b Compare December 2, 2025 01:39
@nicknisi nicknisi merged commit 19b1829 into version-8 Dec 2, 2025
6 checks passed
@nicknisi nicknisi deleted the nicknisi/audit-log-tests branch December 2, 2025 15:43
nicknisi added a commit that referenced this pull request Dec 4, 2025
## Summary
- Use Jest fake timers instead of real timers in audit log retry tests
- Tests now run instantly instead of waiting for actual retry delays
- Removes timeout extensions that were needed for slow tests
nicknisi added a commit that referenced this pull request Dec 4, 2025
## Summary
- Use Jest fake timers instead of real timers in audit log retry tests
- Tests now run instantly instead of waiting for actual retry delays
- Removes timeout extensions that were needed for slow tests
nicknisi added a commit that referenced this pull request Dec 16, 2025
## Summary
- Use Jest fake timers instead of real timers in audit log retry tests
- Tests now run instantly instead of waiting for actual retry delays
- Removes timeout extensions that were needed for slow tests
nicknisi added a commit that referenced this pull request Dec 22, 2025
## Summary
- Use Jest fake timers instead of real timers in audit log retry tests
- Tests now run instantly instead of waiting for actual retry delays
- Removes timeout extensions that were needed for slow tests
nicknisi added a commit that referenced this pull request Jan 8, 2026
## Summary
- Use Jest fake timers instead of real timers in audit log retry tests
- Tests now run instantly instead of waiting for actual retry delays
- Removes timeout extensions that were needed for slow tests
nicknisi added a commit that referenced this pull request Jan 9, 2026
## Summary
- Use Jest fake timers instead of real timers in audit log retry tests
- Tests now run instantly instead of waiting for actual retry delays
- Removes timeout extensions that were needed for slow tests
nicknisi added a commit that referenced this pull request Jan 12, 2026
## Summary
- Use Jest fake timers instead of real timers in audit log retry tests
- Tests now run instantly instead of waiting for actual retry delays
- Removes timeout extensions that were needed for slow tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants