Skip to content

Conversation

joehan
Copy link
Contributor

@joehan joehan commented Aug 20, 2025

Description

From Jules:

Adds unit tests for the timeoutFallback and timeoutError functions in src/timeout.ts.

The tests cover the following scenarios:

  • Promise resolves before timeout
  • Timeout occurs before promise resolves
  • Custom error messages and objects for timeoutError

Adds unit tests for the `timeoutFallback` and `timeoutError` functions in `src/timeout.ts`.

The tests cover the following scenarios:
- Promise resolves before timeout
- Timeout occurs before promise resolves
- Custom error messages and objects for `timeoutError`
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Summary of Changes

Hello @joehan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces comprehensive unit tests for the timeoutFallback and timeoutError utility functions located in src/timeout.ts. The new tests ensure the robustness and correctness of these functions by covering various asynchronous scenarios, including successful promise resolution and different timeout conditions with appropriate error handling or fallback mechanisms.

Highlights

  • New Test File: A new test file, src/timeout.spec.ts, has been added to the project.
  • timeoutFallback Function Tests: Unit tests for the timeoutFallback function have been implemented, verifying its behavior when a promise resolves successfully before the timeout and when the fallback value is returned due to a timeout.
  • timeoutError Function Tests: Unit tests for the timeoutError function have been implemented, covering scenarios where a promise resolves before timeout, and when a timeout occurs, including cases with default error messages, custom error messages, and custom error objects.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds a good set of unit tests for the timeout utility functions. The test cases cover the main scenarios correctly. I've added a couple of suggestions to improve the robustness and reliability of the tests. One suggestion is about using fake timers to avoid flakiness, and the other is about improving type safety in error handling within the tests. Overall, great work on adding test coverage.

Comment on lines +4 to +16
describe("timeoutFallback", () => {
it("should resolve with the promise value when it completes before timeout", async () => {
const promise = new Promise<string>((resolve) => setTimeout(() => resolve("success"), 10));
const result = await timeoutFallback(promise, "fallback", 20);
expect(result).to.equal("success");
});

it("should resolve with the fallback value when timeout occurs", async () => {
const promise = new Promise<string>((resolve) => setTimeout(() => resolve("success"), 30));
const result = await timeoutFallback(promise, "fallback", 20);
expect(result).to.equal("fallback");
});
});
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

These tests rely on setTimeout with real timers, which can sometimes lead to flaky tests on slower environments or under heavy load. Consider using fake timers (e.g., from sinon.js or Jest's built-in fake timers if you were using Jest) to make these tests deterministic and faster. With fake timers, you can control the passage of time manually (e.g., clock.tick(30)) without actually waiting.

Comment on lines +30 to +32
} catch (e: any) {
expect(e.message).to.equal("Operation timed out.");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

While using e: any in a catch block works, it's generally better for type safety to catch the error as unknown (the default in strict mode) and then perform type checks. This makes the test more robust.

This same improvement can be applied to the other rejection tests in this file (lines 41-43 and 52-54).

} catch (e) {
      expect(e).to.be.an.instanceof(Error);
      expect((e as Error).message).to.equal("Operation timed out.");
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant