Repository navigation
Add organized regression suites and fix persistence, privacy, and lifecycle bugs - #912
Merged
Merged
Conversation
Contributor
Dependency ReviewThe following issues were found:
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if (!ReferenceEquals(originalResult, executed.Result) && originalResult is FileStreamResult streamResult) | ||
| { | ||
| try { await streamResult.FileStream.DisposeAsync().ConfigureAwait(false); } | ||
| catch (Exception ex) { _logger.Warning($"Spoiler Guard source stream cleanup failed: {ex.Message}"); } |
4eh5xitv6787h645ebv
added a commit
to 4eh5xitv6787h645ebv/Jellyfin-Enhanced
that referenced
this pull request
Oct 5, 2026
…skipped renderChunkInSlices counted a client-paged chunk as rendered before its cards were built. When a card failed to build, appendInSlices took the chunk's cards out again, but the cursor had already moved past them, so the scroll engine's retry rendered the next chunk and those titles never appeared (a failure in the last chunk also ended pagination). The cursor now advances once the chunk is in, as the synchronous renderChunk does, and only while that render is still current. Found by the regression suite in n00bcodr#912 (perf-round2-discovery test).
…dy created Two instances sharing the config dir could both see no secret file. The second write passed overwrite: File.Exists(path), so it replaced the secret the first had already handed out, invalidating its poster URLs. File.Move without overwrite would not have closed the gap either: on Unix it checks the destination and then renames. Secret creation is now serialised through an exclusive lock file and the secret is re-read under the lock, so a valid secret is never replaced and only a corrupt one is. The concurrency test is deterministic: a test hook forces the loser to see the winner's file appear, and a barrier makes every instance pass the existence check before any of them writes.
… corrupt state - Enable journals each disabled user before touching the next. If that write fails, the user stays on the in-memory restore list, nobody else is disabled, and the next tick or disable restores them. Previously a failed final save lost the whole list. - Disable and expiry fail open: an unwritable state file no longer keeps users locked out. They are restored and anyone who could not be stays pending in memory. Only enable refuses an unwritable journal. - A missing state file is no longer confused with an unreadable or corrupt one. A corrupt file is backed up like reviews.json, is never overwritten, and blocks enabling until it is repaired or removed; an unreadable one is not cached as empty. - Deleted users and users without a policy are dropped from the restore list, and a restore that keeps failing no longer blocks every later window: it is carried into the new window and retried when it ends.
JE.saveUserSettings logs and swallows every failure, so the rollback in
bookmark add, update, delete and sync could never run: a failed save left
the change in memory and reported success.
saveUserSettings takes an opt-in { throwOnError: true } that rejects when
nothing was saved; every other caller keeps the old log-and-resolve
behaviour. The bookmark mutations opt in. The bookmark tests now run the
real saveUserSettings against a failing ApiClient.ajax instead of a
throwing stub.
The runtime combo is built in the editor's Meta+Ctrl+Alt+Shift order, so a binding an admin typed as "Shift+Ctrl+S" stopped matching. Stored combos are now canonicalised to the same modifier order (and letter case) before comparing.
TMDB air dates ("2025-02-01"), movie release dates
("2025-02-01T00:00:00.000Z") and trimmed episode premiere dates are
calendar dates, but they were parsed as UTC midnight and formatted in
local time, so users west of UTC saw the previous day. The leading date
is now formatted as that local calendar day; anything else is parsed as
before. A test runs the chips under America/Los_Angeles.
Under a culture with another calendar (th-TH is Buddhist) the start/end query values were read as Buddhist years and the Shoko range and dedup keys were formatted the same way, so every event fell outside the range. The range is parsed with InvariantCulture (assumed and adjusted to UTC, as before) and the yyyy-MM-dd values are formatted invariantly. A test runs the calendar under th-TH.
…gion alias ICU only promotes baku1926 to the Baku script when no other variant becomes alalc97 (heploc) or a region (aaland -> AX): "az-baku1926-heploc" is "Azerbaijani (ALALC97_BAKU1926)" and "az-baku1926-aaland" is "Azerbaijani (Åland Islands, Unified Turkic Latin Alphabet)". The native resolver promoted both to "(Baku)". The generator now includes CLDR's aaland variant alias, the resolver fills an empty region from it, and promotion is skipped beside those variants. The tags are in the parity corpus (aaland across the variant matrix) and match Node 26.2.0 Intl.DisplayNames.
Sweeps ran on fire-and-forget tasks after a 2 s settle delay, so they could outlive StopAsync and still read and write user files, and the promoter test asserted Bob's state while the sweep could still be running. StopAsync now cancels sweeps that are still settling and waits for a running one to finish. An internal WaitForSweepsAsync lets tests wait for the sweep instead of racing it, and the settle delay is an internal property tests set to zero; production keeps 2 s.
The full-refresh test slept a real 2 s between consecutive award logos (about 20 s per run). The delay is now an internal property, still 2 s in production, that the test sets to zero.
…fail - Auto requests: configure TMDB and Seerr so only the switches can stop a request, count user lookups and HTTP calls, and add an enabled control. - Maintenance: a selection with no valid id must not target everyone. - Seerr tasks: isolate the JellyseerrEnabled master switch for user import and both watchlist syncs, with an enabled control; the missing-credentials test now also enables the Seerr watchlist sync. - Awards: hold the first Wikidata request open so the coalescing test really overlaps 20 callers. - TMDB cache: assert a hit one tick before the 30 minute expiry. - Logger and usage-period tests no longer fail when a run crosses midnight.
… timeouts - load() wraps each file the way Services/ClientScriptBundle.cs does, and a smoke test loads every js/component-scripts.json module in manifest order with that wrapper, so order or file-scope mistakes fail. - Both runners (and frontend mutation replays) pin TZ=UTC and an en_US.UTF-8 locale, and give each test a 20 second timeout. - An expectConsoleError that never matched now fails teardown; allowConsoleError covers diagnostics that may or may not occur.
- Reviews: Season/Episode fixtures carry their indices, the guard only matches the right key, and an unguarded control must fetch. - Quota: count fetches while disabled (zero) and with a control. - Release dates: the detached placeholder must stay empty. - Auto-skip: the current item's segments land before the stale ones. - More info: count detail fetches after close to catch a leaked listener. - Playback: the item-identity test no longer claims an OSD fallback.
The fixture server already read JE_BROWSER_PORT, but the config hard-coded 4179 for the readiness URL and baseURL. The port is read once and used for the server environment, its readiness probe and every test URL.
…nets Cleanup only ran when create had returned, so a docker run that created the container and then failed, or a run interrupted mid-create, leaked it. Cleanup now always removes the container and network by their unique name and treats "no such" as done. Both carry a je-regression.run label, and network creation moves to the next free subnet when a concurrent run takes the chosen one.
Moq 4.20.72 (BSD-3-Clause) is referenced only by tests/backend and failed the license allow-list. It is excluded by package URL, which keeps BSD-3-Clause disallowed for everything else. It was the only package the review flagged.
The inventory recorded line counts, line numbers and locale key counts, so almost any production edit or Weblate update made it stale. Because the check ran first in the backend job, it also skipped the backend tests. The inventory now only changes when a file, route, setting, scheduled task, locale file or storage literal is added, removed or renamed; the check runs as its own job; and CONTRIBUTING explains how to regenerate it.
The audit installed only the .NET 9 SDK, so restoring the net10 default target failed and the grep for vulnerable packages passed on empty output. It now installs .NET 9 and 10, restores and lists jf12 and jf10 separately with pipefail, and fails on any restore error, reported problem or vulnerable package in the JSON output.
…ents The backend floors (38% lines / 27% branches) sat ten points under the measured 48.3% / 36.3% on both targets, so they could not catch a real drop; they are now 47% / 35%. The frontend floors move from 20/15/21/21 to 28/20/29/30 (statements/branches/functions/lines) against the measured 29.5/21.3/30.3/31.0. The poster harness project comments no longer claim CI never builds them.
progress.md and perf-round2.md were session logs that are now false (no commits, HEAD 8b8a5ba, no GitHub Actions run) and reported on a private fork branch; they are removed. The discovery-pagination regression test they described stays. validation.md is rewritten as the commands to run and the latest results; the coverage matrix and testing README point to it, and the counts, inventory figures and coverage floors match the final run. Evidence no longer cites ignored artifacts/ or /tmp paths.
Comment on lines
+625
to
+629
| catch (Exception ex) | ||
| { | ||
| ReportLoadFailure($"could not read maintenance-state.json: {ex.Message}", backup: false); | ||
| return new MaintenanceState(); | ||
| } |
Comment on lines
+636
to
+639
| catch (Exception ex) | ||
| { | ||
| parseError = ex.Message; | ||
| } |
Comment on lines
+669
to
+672
| catch (Exception) | ||
| { | ||
| // The message alone still deduplicates. | ||
| } |
Main now sizes the Requests, Issues and History pages to whole grid rows instead of a fixed 20. Stub the live column count, pre-set each page size and assert skip/take for all three lists, then check that a column change re-sizes the page, returns to page 1 and keeps History under its 48-card cap.
A save serialises the whole bookmark map when it is called, but its POST waits behind earlier saves. When two mutations overlapped and the first save failed, its rollback could bring back a record the second save had already removed on the server, or drop one the server kept, and the next save made that permanent. Queue each user's add, update, delete and sync so one mutates, saves and rolls back before the next touches memory, and re-check the owner when a queued mutation starts so it never writes for a user who has signed out.
The replacement migration deleted the old group from memory before syncing, and the sync rollback only removed the new copies, so a failed save lost the group and the next successful save persisted the loss. syncBookmarks now takes a replaceOriginals option that removes the old records in the same save and restores them if it fails; the migration uses it.
…w the intensity The filter tests only checked that the bytes changed, so serving a re-encoded unblurred image or ignoring SpoilerBlurIntensity passed. Decode the result and require most of the checkerboard's contrast to be gone in blur mode and the hide-mode fallback, and a higher intensity to remove more.
Add a second movie whose id starts with the first one's digits and whose status also changed. It must get its own status and request its own id, which a type-only or prefix id match would break.
The bundle assertion reused the controller whose Cache-Control GetMainScript had already set, so the bundle route could skip its header. Use a fresh controller and require each dev-mode request to rebuild the bundle.
The oldest entry was also the lowest key and the first inserted, so pruning by key or insertion order passed. Give the oldest timestamp to the key that sorts last and is inserted last.
…sized list The oversized input was also entirely invalid, so dropping the length guard still failed it. Pad a valid list to the 2048-character limit and one past it, and check the length error.
…rder Each release type appeared in one country only, so swapping the region order or dropping it passed. List one type in GB first, then US and AU.
Only the admin delete button was checked on another user's review, so treating every review as the viewer's own passed. Require no edit or delete controls there and both on the viewer's own review.
…lture DateTime.TryParse used the server culture, so under fa-IR a released "2001-01-01" read as a Persian year (2622) and the next collection movie was never requested; under th-TH a future date read as a Thai Buddhist year and was requested early. Parse with the invariant culture as UTC and compare against UtcNow.
…culture TMDB person birthdays/deathdays and the Seerr air/release dates behind the Coming Soon filter and sort are date-only ISO strings, which a culture-sensitive parse reads in the server calendar (Persian, Thai Buddhist). Person birth/death dates were also formatted in that calendar before the client recomputed ages from them. Seerr createdAt parses are pinned to the invariant culture as well.
The clock was installed running at real speed, so a runner stall over 1.3 s expired the toast before its first assertion, and removing the expiry runFor still passed. Pause it before the toast exists and check the toast is still there just before removal and gone just after.
Both language tests installed a real-speed clock, so the panel's 1.5 s and 2 s location.reload timers still fired on a slow runner and wiped the page state the tests read. Pause the clock instead.
The visual test promised labelled controls but only located the season select by id. Find it, and the Request and Cancel buttons, by their accessible names inside the dialog.
The inventory generator hard-coded /JellyfinEnhanced/ in front of every action template, so the Catch Up controller's routes (class route JellyfinEnhanced/catchup) were recorded without their catchup segment. Use the class-level [Route] of the controller that declares each action, falling back to JellyfinEnhanced, and add a tooling test with a custom route, a nested class and a controller without a route. Regenerate the inventory for the merged Catch Up and Plugin Pages files and update the coverage matrix counts.
Catch Up came in from main with no behavioral tests. Add its row as Unverified, naming the only generic checks that reach it (the manifest smoke tests and the inventory), add its log to the shared stores and list it under the remaining gaps.
Round 7 pinned the collection release-date parse to the invariant culture but also moved the cutoff to UTC midnight, so a server west of UTC requested a sequel hours before its release day began and one east of UTC waited hours into it. Keep the invariant-culture parse, read the date-only value as local midnight and compare it with the local clock, as before the culture fix. The culture test now shares its setup with a new check that a title released today in the server's time zone is requested.
Rerun every listed stage at b8179d9 and replace the table, which had mixed results from earlier rounds: 10 tooling checks, 804 backend tests per target, the new coverage figures, and the backend reruns under the fa-IR culture and the America/Los_Angeles time zone.
4eh5xitv6787h645ebv
marked this pull request as ready for review
October 8, 2026 09:31
Replace the digest pins with the floating jellyfin/jellyfin:10.11 and jellyfin/jellyfin:12 tags (never latest, which would jump to 13). Each target pulls its tag before running and fails if the pull fails rather than using a cached image, then runs the exact pulled digest. report.json keeps image (now the tag) and adds resolved_image (repo digest) and server_version (from /System/Info/Public); the run log prints both. Docs describe the new behaviour.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This adds a unified regression-testing system for JE, and fixes the persistence, privacy, lifecycle and locale bugs the tests exposed. Tests, shared fixtures, the existing poster harnesses (moved from
scripts/), runners and documentation all live undertests/.python3 tests/run.py allruns everything.Production fixes
Maintenance mode
Spoiler Guard and privacy
srcset.Poster tags
aalandand script variants.Bookmarks
Dates and locales
Caches and stored data
reviews.jsondata is handled without breaking the load.Seerr and watchlist
POST jellyseerr/sync-watchlistnow runs the scheduled task's pass:502when none of them answers.Smaller fixes
Validation
All stages passed in one local run at a single commit:
GitHub Actions passes on the PR. The host and extended jobs run only on schedule or manual dispatch.
tests/docs/validation.mdhas the exact commands and the full results table.tests/docs/README.mdlists the prerequisites: the .NET runtimes, Node version, Playwright browsers and native Linux Docker.Remaining coverage
This is broad regression coverage, not exhaustive behavioural coverage.
tests/docs/coverage-matrix.mdlists the gaps. They include real-host playback and native-image journeys, untested feature combinations, the Catch Up page added onmain, and a short list of guards no test reaches yet.The suite also includes a discovery-pagination retry regression test (
tests/frontend/regressions/perf-round2-discovery.test.mjs). It passes on this branch.