Skip to content

test: add scanner update rate override for testing #3241

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
michaelhthomas wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

test: add scanner update rate override for testing #3241

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bf7de2a
test: add scanner update rate override for testing
michaelhthomas Jul 12, 2026
160f931
test: use mock timers instead of scanner test overrides
michaelhthomas Aug 16, 2026
2e98009
fix(runWithMockTimers): fail when timer guard is exhausted
michaelhthomas Aug 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions server/lib/scanners/baseScanner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ export interface ProcessableSeason {
}

class BaseScanner<T> {
private bundleSize;
private updateRate;
protected bundleSize;
protected updateRate;

@fallenbagel fallenbagel Aug 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Imo, no need to widen these. setTestOverrides is a method on this class, it already has access to the private fields. Widening also makes protectedBundleSize/protectedUpdateRate below redundant for subclasses, now there's two ways to read the same state.

protected progress = 0;
protected items: T[] = [];
protected totalSize?: number = 0;
Expand Down Expand Up @@ -792,6 +792,18 @@ class BaseScanner<T> {
get protectedBundleSize(): number {
return this.bundleSize;
}

/**
* Test-only override to reduce scan delay in unit tests.
* Sets updateRate and/or bundleSize for the next run.
*/
public setTestOverrides(opts: {
updateRate?: number;
bundleSize?: number;
}): void {
if (opts.updateRate !== undefined) this.updateRate = opts.updateRate;
if (opts.bundleSize !== undefined) this.bundleSize = opts.bundleSize;
}

@coderabbitai coderabbitai Bot Jul 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Validate override values before mutating scanner state.

A bundleSize of 0 causes loop() to recurse with the same start forever because start + this.bundleSize never advances. Reject non-positive/non-integer bundle sizes and negative or non-finite update rates.

Proposed validation 
   public setTestOverrides(opts: {
     updateRate?: number;
     bundleSize?: number;
   }): void {
+    if (
+      opts.updateRate !== undefined &&
+      (!Number.isFinite(opts.updateRate) || opts.updateRate < 0)
+    ) {
+      throw new RangeError('updateRate must be a finite, non-negative number');
+    }
+    if (
+      opts.bundleSize !== undefined &&
+      (!Number.isInteger(opts.bundleSize) || opts.bundleSize <= 0)
+    ) {
+      throw new RangeError('bundleSize must be a positive integer');
+    }
     if (opts.updateRate !== undefined) this.updateRate = opts.updateRate;
     if (opts.bundleSize !== undefined) this.bundleSize = opts.bundleSize;
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* Test-only override to reduce scan delay in unit tests.
* Sets updateRate and/or bundleSize for the next run.
*/
public setTestOverrides(opts: {
updateRate?: number;
bundleSize?: number;
}): void {
if (opts.updateRate !== undefined) this.updateRate = opts.updateRate;
if (opts.bundleSize !== undefined) this.bundleSize = opts.bundleSize;
}
/**
* Test-only override to reduce scan delay in unit tests.
* Sets updateRate and/or bundleSize for the next run.
*/
public setTestOverrides(opts: {
updateRate?: number;
bundleSize?: number;
}): void {
if (
opts.updateRate !== undefined &&
(!Number.isFinite(opts.updateRate) || opts.updateRate < 0)
) {
throw new RangeError('updateRate must be a finite, non-negative number');
}
if (
opts.bundleSize !== undefined &&
(!Number.isInteger(opts.bundleSize) || opts.bundleSize <= 0)
) {
throw new RangeError('bundleSize must be a positive integer');
}
if (opts.updateRate !== undefined) this.updateRate = opts.updateRate;
if (opts.bundleSize !== undefined) this.bundleSize = opts.bundleSize;
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/lib/scanners/baseScanner.ts` around lines 795 - 806, Update
BaseScanner.setTestOverrides to validate values before mutating scanner state:
reject bundleSize values that are non-positive or non-integer, and reject
updateRate values that are negative or non-finite. Preserve existing state when
validation fails, and only assign validated overrides.

@fallenbagel fallenbagel Aug 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a public, unguarded mutator on a class with live singleton instances (radarrScanner, sonarrScanner, jellyfinFullScanner). Nothing stops it from being called outside tests, unlike protectedBundleSize/protectedUpdateRate above which are read-only. I'd rather drop the source change and use mock.timers in the three test files instead since mock is already imported there for mock.method, so mock.timers.enable() + mock.timers.tick(this.updateRate) gets the same speedup and with the added bonus of zero production code changes.

CC: @seerr-team/seerr-core

@michaelhthomas michaelhthomas Aug 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Lowkey didn't realize the Node test runner supported mock timers. I will just go with that, should be a lot simpler.

}

export default BaseScanner;
2 changes: 2 additions & 0 deletions server/lib/scanners/jellyfin/jellyfin.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,8 @@ function configureJellyfinWithLibrary(

describe('Jellyfin Scanner', () => {
beforeEach(async () => {
jellyfinFullScanner.setTestOverrides({ updateRate: 0 });

@fallenbagel fallenbagel Aug 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same as the Radarr test


getLibraryContentsImpl = async () => [];
getItemDataImpl = async () => undefined;
getSeasonsImpl = async () => [];
Expand Down
1 change: 1 addition & 0 deletions server/lib/scanners/radarr/radarr.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ function fakeRadarrMovie(overrides: Partial<RadarrMovie> = {}): RadarrMovie {

describe('Radarr Scanner', () => {
beforeEach(() => {
radarrScanner.setTestOverrides({ updateRate: 0 });

@fallenbagel fallenbagel Aug 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same concern as stated below. Would rather see mock.timers.enable({ apis: ['setTimeout'] }) in beforeEach and a mock.timers.tick(4000) after triggering the scan, gets the same speedup without touching baseScanner.ts. One tick covers everything currently mocked in this file and bundleSize is 50 (20 for Jellyfin) and nothing here mocks more than a handful of items. So this only needs repeating if a test starts mocking more than a bundle's worth.

getMoviesImpl = async () => [];
});

Expand Down
1 change: 1 addition & 0 deletions server/lib/scanners/sonarr/sonarr.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ function configureSonarr(overrides: Partial<SonarrSettings>[] = [{}]): void {

describe('Sonarr Scanner', () => {
beforeEach(() => {
sonarrScanner.setTestOverrides({ updateRate: 0 });

@fallenbagel fallenbagel Aug 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same as the Radarr test

getSeriesImpl = async () => [];
getShowByTvdbIdImpl = async () => fakeTmdbShow(1);
getTvShowImpl = async () => fakeTmdbShow(1);
Expand Down
Loading