[PR #2850] [CLOSED] Fix warnings in Jellyfin.Naming.Tests #9249

Closed
opened 2026-02-07 05:58:48 +03:00 by OVERLORD · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/jellyfin/jellyfin/pull/2850
Author: @dtparr
Created: 4/13/2020
Status: Closed

Base: masterHead: namingTestsWarnings


📝 Commits (2)

  • 8d0f29d Fix warnings in Jellyfin.Naming.Tests
  • 1d894fd Fix comment spacing per review

📊 Changes

10 files changed (+54 additions, -21 deletions)

View changed files

📝 tests/Jellyfin.Naming.Tests/Subtitles/SubtitleParserTests.cs (+1 -1)
📝 tests/Jellyfin.Naming.Tests/TV/AbsoluteEpisodeNumberTests.cs (+2 -1)
📝 tests/Jellyfin.Naming.Tests/TV/DailyEpisodeTests.cs (+7 -6)
📝 tests/Jellyfin.Naming.Tests/TV/EpisodeNumberWithoutSeasonTests.cs (+2 -1)
📝 tests/Jellyfin.Naming.Tests/TV/EpisodeWithoutSeasonTests.cs (+4 -3)
📝 tests/Jellyfin.Naming.Tests/TV/SeasonNumberTests.cs (+2 -1)
📝 tests/Jellyfin.Naming.Tests/TV/SimpleEpisodeTests.cs (+4 -3)
📝 tests/Jellyfin.Naming.Tests/Video/Format3DTests.cs (+4 -3)
📝 tests/Jellyfin.Naming.Tests/Video/StubTests.cs (+3 -2)
📝 tests/Jellyfin.Naming.Tests/Video/VideoResolverTests.cs (+25 -0)

📄 Description

This PR removes all the warnings in Jellyfin.Naming.Tests other than the ones related to not marking methods as [Fact] (I tested a couple of those, and they did indeed fail when [Fact] was uncommented, so there are presumably fixes to the naming functionality required to get those warnings to go away).

Changes
Most of the issues revolved around potential nulls. I added Assert.NotNull's in those places to protect against a bare null dereference. Unfortuantely, while the Xunit has code on master to have anything passed into an Assert.NotNull treated as not nullable afterwards, that version of Xunit has not been released yet, so that doesn't satisfy the compiler. For cases where there were only a handful of these per test class, I simply tagged the object with ! (e.g. result!.foo) thereafter, and those can be removed in the long run. For one test class that had several dozen cases, I created a utility method to wrap the Xunit Assert.NotNull with an annotated method to satisfy the compiler without the need for all the !'s splashed from place to place.

Issues
This is related to #2149, although it will not resolve the Jellyfin.Naming.Tests entirely due to the above.


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/jellyfin/jellyfin/pull/2850 **Author:** [@dtparr](https://github.com/dtparr) **Created:** 4/13/2020 **Status:** ❌ Closed **Base:** `master` ← **Head:** `namingTestsWarnings` --- ### 📝 Commits (2) - [`8d0f29d`](https://github.com/jellyfin/jellyfin/commit/8d0f29d1fb1e1ab53e394fd823524c6d0065cf6f) Fix warnings in Jellyfin.Naming.Tests - [`1d894fd`](https://github.com/jellyfin/jellyfin/commit/1d894fdba0bc074c6729a61705ae672fdd03842b) Fix comment spacing per review ### 📊 Changes **10 files changed** (+54 additions, -21 deletions) <details> <summary>View changed files</summary> 📝 `tests/Jellyfin.Naming.Tests/Subtitles/SubtitleParserTests.cs` (+1 -1) 📝 `tests/Jellyfin.Naming.Tests/TV/AbsoluteEpisodeNumberTests.cs` (+2 -1) 📝 `tests/Jellyfin.Naming.Tests/TV/DailyEpisodeTests.cs` (+7 -6) 📝 `tests/Jellyfin.Naming.Tests/TV/EpisodeNumberWithoutSeasonTests.cs` (+2 -1) 📝 `tests/Jellyfin.Naming.Tests/TV/EpisodeWithoutSeasonTests.cs` (+4 -3) 📝 `tests/Jellyfin.Naming.Tests/TV/SeasonNumberTests.cs` (+2 -1) 📝 `tests/Jellyfin.Naming.Tests/TV/SimpleEpisodeTests.cs` (+4 -3) 📝 `tests/Jellyfin.Naming.Tests/Video/Format3DTests.cs` (+4 -3) 📝 `tests/Jellyfin.Naming.Tests/Video/StubTests.cs` (+3 -2) 📝 `tests/Jellyfin.Naming.Tests/Video/VideoResolverTests.cs` (+25 -0) </details> ### 📄 Description This PR removes all the warnings in Jellyfin.Naming.Tests other than the ones related to not marking methods as [Fact] (I tested a couple of those, and they did indeed fail when [Fact] was uncommented, so there are presumably fixes to the naming functionality required to get those warnings to go away). **Changes** Most of the issues revolved around potential nulls. I added Assert.NotNull's in those places to protect against a bare null dereference. Unfortuantely, while the Xunit has code on master to have anything passed into an Assert.NotNull treated as not nullable afterwards, that version of Xunit has not been released yet, so that doesn't satisfy the compiler. For cases where there were only a handful of these per test class, I simply tagged the object with ! (e.g. result!.foo) thereafter, and those can be removed in the long run. For one test class that had several dozen cases, I created a utility method to wrap the Xunit Assert.NotNull with an annotated method to satisfy the compiler without the need for all the !'s splashed from place to place. **Issues** This is related to #2149, although it will not resolve the Jellyfin.Naming.Tests entirely due to the above. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
OVERLORD added the pull-request label 2026-02-07 05:58:48 +03:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/jellyfin#9249