Skip to content

Version 4.5#21

Open
souphan-adsk wants to merge 5 commits into
masterfrom
version-4.4
Open

Version 4.5#21
souphan-adsk wants to merge 5 commits into
masterfrom
version-4.4

Conversation

@souphan-adsk

@souphan-adsk souphan-adsk commented Dec 8, 2025

Copy link
Copy Markdown
Contributor

Introduces a testing hook in the retry mechanism. Mainly to support fake timer advancement during tests.

@souphan-adsk souphan-adsk requested a review from Copilot December 8, 2025 21:16

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a testing hook in the retry mechanism for the ShotgunApiClient to support fake timer advancement during tests.

Key Changes:

  • Added an onRetry callback handler to the retry configuration that allows tests to control timer behavior during retry operations

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/client.js
Comment thread src/client.js Outdated
Comment on lines +209 to +211
// Wait for next tick to ensure async-retry has set up setTimeout
if (this.onAfterSleepSetup) {
await Promise.resolve(); // Let async-retry set up setTimeout first

Copilot AI Dec 8, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] The comment explains the technical detail but doesn't clarify why this is necessary for the testing hook to work correctly. Consider expanding the comment to explain that without this tick, the fake timer might not be ready when onAfterSleepSetup is called.

Suggested change
// Wait for next tick to ensure async-retry has set up setTimeout
if (this.onAfterSleepSetup) {
await Promise.resolve(); // Let async-retry set up setTimeout first
// Wait for next tick to ensure async-retry has set up setTimeout.
// This is necessary because, in test environments using fake timers,
// calling onAfterSleepSetup too early (before setTimeout is set up)
// can cause the fake timer not to be ready, leading to unreliable tests.
if (this.onAfterSleepSetup) {
await Promise.resolve(); // Ensures setTimeout is set up before calling the hook

Copilot uses AI. Check for mistakes.
@souphan-adsk souphan-adsk changed the title Version 4.4 Version 4.5 Dec 8, 2025

@francoisjacques francoisjacques left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM!

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.

3 participants