From bd374825f19063091a62b37f7e5a359ce577c0e3 Mon Sep 17 00:00:00 2001 From: Qstick Date: Sat, 28 Sep 2019 22:39:09 -0400 Subject: [PATCH] Fixed: Logging Statements and Tests --- .../DiskTests/DiskProviderFixtureBase.cs | 2 +- .../FirstCharcacterToUpperFixture.cs | 18 ++- .../SentryTargetFixture.cs | 85 +++++++++++++ src/NzbDrone.Common.Test/WebClientTests.cs | 20 +-- .../NotificationBaseFixture.cs | 120 ++++++++++++++++++ .../Qualities/QualityFinderFixture.cs | 39 ++++++ .../Housekeepers/DeleteBadMediaCovers.cs | 2 +- .../Housekeeping/HousekeepingService.cs | 2 +- .../Aggregators/AggregateQuality.cs | 2 +- .../Messaging/Commands/CommandExecutor.cs | 6 +- .../Messaging/Commands/CommandQueueManager.cs | 1 - src/NzbDrone.Core/Qualities/Quality.cs | 2 +- src/NzbDrone.Core/Qualities/QualityFinder.cs | 6 +- src/NzbDrone.Core/Qualities/QualitySource.cs | 13 -- 14 files changed, 276 insertions(+), 42 deletions(-) create mode 100644 src/NzbDrone.Common.Test/InstrumentationTests/SentryTargetFixture.cs create mode 100644 src/NzbDrone.Core.Test/NotificationTests/NotificationBaseFixture.cs create mode 100644 src/NzbDrone.Core.Test/Qualities/QualityFinderFixture.cs delete mode 100644 src/NzbDrone.Core/Qualities/QualitySource.cs diff --git a/src/NzbDrone.Common.Test/DiskTests/DiskProviderFixtureBase.cs b/src/NzbDrone.Common.Test/DiskTests/DiskProviderFixtureBase.cs index f02b07d7a..c81aefbd8 100644 --- a/src/NzbDrone.Common.Test/DiskTests/DiskProviderFixtureBase.cs +++ b/src/NzbDrone.Common.Test/DiskTests/DiskProviderFixtureBase.cs @@ -33,7 +33,6 @@ namespace NzbDrone.Common.Test.DiskTests protected abstract void SetWritePermissions(string path, bool writable); [Test] - [Retry(5)] public void FolderWritable_should_return_true_for_writable_directory() { var tempFolder = GetTempFilePath(); @@ -45,6 +44,7 @@ namespace NzbDrone.Common.Test.DiskTests } [Test] + [Retry(5)] public void FolderWritable_should_return_false_for_unwritable_directory() { var tempFolder = GetTempFilePath(); diff --git a/src/NzbDrone.Common.Test/ExtensionTests/StringExtensionTests/FirstCharcacterToUpperFixture.cs b/src/NzbDrone.Common.Test/ExtensionTests/StringExtensionTests/FirstCharcacterToUpperFixture.cs index 53e221f4e..5013ddd73 100644 --- a/src/NzbDrone.Common.Test/ExtensionTests/StringExtensionTests/FirstCharcacterToUpperFixture.cs +++ b/src/NzbDrone.Common.Test/ExtensionTests/StringExtensionTests/FirstCharcacterToUpperFixture.cs @@ -1,4 +1,5 @@ -using FluentAssertions; +using System.Globalization; +using FluentAssertions; using NUnit.Framework; using NzbDrone.Common.Extensions; @@ -16,5 +17,20 @@ namespace NzbDrone.Common.Test.ExtensionTests.StringExtensionTests { input.FirstCharToUpper().Should().Be(expected); } + + [Test] + public void should_capitalize_first_character_regardless_of_culture() + { + var current = CultureInfo.CurrentCulture; + CultureInfo.CurrentCulture = CultureInfo.GetCultureInfo("tr-TR"); + try + { + "infInite".FirstCharToUpper().Should().Be("InfInite"); + } + finally + { + CultureInfo.CurrentCulture = current; + } + } } } diff --git a/src/NzbDrone.Common.Test/InstrumentationTests/SentryTargetFixture.cs b/src/NzbDrone.Common.Test/InstrumentationTests/SentryTargetFixture.cs new file mode 100644 index 000000000..70c701b74 --- /dev/null +++ b/src/NzbDrone.Common.Test/InstrumentationTests/SentryTargetFixture.cs @@ -0,0 +1,85 @@ +using NUnit.Framework; +using FluentAssertions; +using NzbDrone.Common.Instrumentation.Sentry; +using System; +using NLog; +using NzbDrone.Test.Common; +using System.Globalization; +using System.Linq; + +namespace NzbDrone.Common.Test.InstrumentationTests +{ + [TestFixture] + public class SentryTargetFixture : TestBase + { + private SentryTarget Subject; + + private static LogLevel[] AllLevels = LogLevel.AllLevels.ToArray(); + private static LogLevel[] SentryLevels = LogLevel.AllLevels.Where(x => x >= LogLevel.Error).ToArray(); + private static LogLevel[] OtherLevels = AllLevels.Except(SentryLevels).ToArray(); + + private static Exception[] FilteredExceptions = new Exception[] { + new UnauthorizedAccessException(), + new TinyIoC.TinyIoCResolutionException(typeof(string)) + }; + + [SetUp] + public void Setup() + { + Subject = new SentryTarget("https://aaaaaaaaaaaaaaaaaaaaaaaaaa@sentry.io/111111"); + } + + private LogEventInfo GivenLogEvent(LogLevel level, Exception ex, string message) + { + return LogEventInfo.Create(level, "SentryTest", ex, CultureInfo.InvariantCulture, message); + } + + [Test, TestCaseSource("AllLevels")] + public void log_without_error_is_not_sentry_event(LogLevel level) + { + Subject.IsSentryMessage(GivenLogEvent(level, null, "test")).Should().BeFalse(); + } + + [Test, TestCaseSource("SentryLevels")] + public void error_or_worse_with_exception_is_sentry_event(LogLevel level) + { + Subject.IsSentryMessage(GivenLogEvent(level, new Exception(), "test")).Should().BeTrue(); + } + + [Test, TestCaseSource("OtherLevels")] + public void less_than_error_with_exception_is_not_sentry_event(LogLevel level) + { + Subject.IsSentryMessage(GivenLogEvent(level, new Exception(), "test")).Should().BeFalse(); + } + + [Test, TestCaseSource("FilteredExceptions")] + public void should_filter_event_for_filtered_exception_types(Exception ex) + { + var log = GivenLogEvent(LogLevel.Error, ex, "test"); + Subject.IsSentryMessage(log).Should().BeFalse(); + } + + [Test, TestCaseSource("FilteredExceptions")] + public void should_not_filter_event_for_filtered_exception_types_if_filtering_disabled(Exception ex) + { + Subject.FilterEvents = false; + var log = GivenLogEvent(LogLevel.Error, ex, "test"); + Subject.IsSentryMessage(log).Should().BeTrue(); + } + + [Test, TestCaseSource(typeof(SentryTarget), "FilteredExceptionMessages")] + public void should_filter_event_for_filtered_exception_messages(string message) + { + var log = GivenLogEvent(LogLevel.Error, new Exception("aaaaaaa" + message + "bbbbbbb"), "test"); + Subject.IsSentryMessage(log).Should().BeFalse(); + } + + [TestCase("A message that isn't filtered")] + [TestCase("Error")] + public void should_not_filter_event_for_exception_messages_that_are_not_filtered(string message) + { + var log = GivenLogEvent(LogLevel.Error, new Exception(message), "test"); + Subject.IsSentryMessage(log).Should().BeTrue(); + } + } +} \ No newline at end of file diff --git a/src/NzbDrone.Common.Test/WebClientTests.cs b/src/NzbDrone.Common.Test/WebClientTests.cs index 34a9de418..46a3561c8 100644 --- a/src/NzbDrone.Common.Test/WebClientTests.cs +++ b/src/NzbDrone.Common.Test/WebClientTests.cs @@ -1,4 +1,5 @@ -using System; + +using System; using FluentAssertions; using NUnit.Framework; using NzbDrone.Common.Http; @@ -19,23 +20,8 @@ namespace NzbDrone.Common.Test } [TestCase("")] - public void DownloadString_should_throw_on_empty_string(string url) - { - Assert.Throws(() => Subject.DownloadString(url)); - ExceptionVerification.ExpectedWarns(1); - } - - // .net 4.6.2 throws NotSupportedException instead of ArgumentException here - [TestCase("http://")] - public void DownloadString_should_throw_on_not_supported_string_windows(string url) - { - WindowsOnly(); - Assert.Throws(() => Subject.DownloadString(url)); - ExceptionVerification.ExpectedWarns(1); - } - [TestCase("http://")] - public void DownloadString_should_throw_on_not_supported_string_mono(string url) + public void DownloadString_should_throw_on_error(string url) { Action action = () => Subject.DownloadString(url); action.Should().Throw(); diff --git a/src/NzbDrone.Core.Test/NotificationTests/NotificationBaseFixture.cs b/src/NzbDrone.Core.Test/NotificationTests/NotificationBaseFixture.cs new file mode 100644 index 000000000..f194b286f --- /dev/null +++ b/src/NzbDrone.Core.Test/NotificationTests/NotificationBaseFixture.cs @@ -0,0 +1,120 @@ +using System; +using FluentAssertions; +using FluentValidation.Results; +using NUnit.Framework; +using NzbDrone.Core.Notifications; +using NzbDrone.Core.ThingiProvider; +using NzbDrone.Core.Movies; +using NzbDrone.Core.Validation; +using NzbDrone.Test.Common; + +namespace NzbDrone.Core.Test.NotificationTests +{ + [TestFixture] + public class NotificationBaseFixture : TestBase + { + class TestSetting : IProviderConfig + { + public NzbDroneValidationResult Validate() + { + return new NzbDroneValidationResult(); + } + } + + class TestNotificationWithOnDownload : NotificationBase + { + public override string Name => "TestNotification"; + public override string Link => ""; + + + public override ValidationResult Test() + { + throw new NotImplementedException(); + } + + public override void OnDownload(DownloadMessage downloadMessage) + { + TestLogger.Info("OnDownload was called"); + } + + } + + class TestNotificationWithAllEvents : NotificationBase + { + public override string Name => "TestNotification"; + public override string Link => ""; + + + public override ValidationResult Test() + { + throw new NotImplementedException(); + } + + public override void OnGrab(GrabMessage grabMessage) + { + TestLogger.Info("OnGrab was called"); + } + + public override void OnDownload(DownloadMessage message) + { + TestLogger.Info("OnDownload was called"); + } + + public override void OnMovieRename(Movie movie) + { + TestLogger.Info("OnRename was called"); + } + + } + + class TestNotificationWithNoEvents : NotificationBase + { + public override string Name => "TestNotification"; + public override string Link => ""; + + + public override ValidationResult Test() + { + throw new NotImplementedException(); + } + + + } + + [Test] + public void should_support_OnUpgrade_should_link_to_OnDownload() + { + var notification = new TestNotificationWithOnDownload(); + + notification.SupportsOnDownload.Should().BeTrue(); + notification.SupportsOnUpgrade.Should().BeTrue(); + + notification.SupportsOnGrab.Should().BeFalse(); + notification.SupportsOnRename.Should().BeFalse(); + } + + [Test] + public void should_support_all_if_implemented() + { + var notification = new TestNotificationWithAllEvents(); + + notification.SupportsOnGrab.Should().BeTrue(); + notification.SupportsOnDownload.Should().BeTrue(); + notification.SupportsOnUpgrade.Should().BeTrue(); + notification.SupportsOnRename.Should().BeTrue(); + } + + + [Test] + public void should_support_none_if_none_are_implemented() + { + var notification = new TestNotificationWithNoEvents(); + + notification.SupportsOnGrab.Should().BeFalse(); + notification.SupportsOnDownload.Should().BeFalse(); + notification.SupportsOnUpgrade.Should().BeFalse(); + notification.SupportsOnRename.Should().BeFalse(); + } + } + +} diff --git a/src/NzbDrone.Core.Test/Qualities/QualityFinderFixture.cs b/src/NzbDrone.Core.Test/Qualities/QualityFinderFixture.cs new file mode 100644 index 000000000..abaa44586 --- /dev/null +++ b/src/NzbDrone.Core.Test/Qualities/QualityFinderFixture.cs @@ -0,0 +1,39 @@ +using FluentAssertions; +using NUnit.Framework; +using NzbDrone.Core.CustomFormats; +using NzbDrone.Core.Qualities; + +namespace NzbDrone.Core.Test.Qualities +{ + [TestFixture] + public class QualityFinderFixture + { + [TestCase(Source.TV, 480, Modifier.NONE)] + [TestCase(Source.UNKNOWN, 480, Modifier.NONE)] + public void should_return_SDTV(Source source, Resolution resolution, Modifier modifier) + { + QualityFinder.FindBySourceAndResolution(source, resolution, modifier).Should().Be(Quality.SDTV); + } + + [TestCase(Source.TV, 720, Modifier.NONE)] + [TestCase(Source.UNKNOWN, 720, Modifier.NONE)] + public void should_return_HDTV_720p(Source source, Resolution resolution, Modifier modifier) + { + QualityFinder.FindBySourceAndResolution(source, resolution, modifier).Should().Be(Quality.HDTV720p); + } + + [TestCase(Source.TV, 1080, Modifier.NONE)] + [TestCase(Source.UNKNOWN, 1080, Modifier.NONE)] + public void should_return_HDTV_1080p(Source source, Resolution resolution, Modifier modifier) + { + QualityFinder.FindBySourceAndResolution(source, resolution, modifier).Should().Be(Quality.HDTV1080p); + } + + [TestCase(Source.BLURAY, 720, Modifier.NONE)] + [TestCase(Source.DVD, 720, Modifier.NONE)] + public void should_return_Bluray720p(Source source, Resolution resolution, Modifier modifier) + { + QualityFinder.FindBySourceAndResolution(source, resolution, modifier).Should().Be(Quality.Bluray720p); + } + } +} \ No newline at end of file diff --git a/src/NzbDrone.Core/Housekeeping/Housekeepers/DeleteBadMediaCovers.cs b/src/NzbDrone.Core/Housekeeping/Housekeepers/DeleteBadMediaCovers.cs index cc1b13f4c..0b9119425 100644 --- a/src/NzbDrone.Core/Housekeeping/Housekeepers/DeleteBadMediaCovers.cs +++ b/src/NzbDrone.Core/Housekeeping/Housekeepers/DeleteBadMediaCovers.cs @@ -54,7 +54,7 @@ namespace NzbDrone.Core.Housekeeping.Housekeepers } catch (Exception e) { - _logger.Error(e, "Couldn't validate image " + image.RelativePath); + _logger.Error(e, "Couldn't validate image {0}", image.RelativePath); } } } diff --git a/src/NzbDrone.Core/Housekeeping/HousekeepingService.cs b/src/NzbDrone.Core/Housekeeping/HousekeepingService.cs index c36ace89d..eb8cedd9e 100644 --- a/src/NzbDrone.Core/Housekeeping/HousekeepingService.cs +++ b/src/NzbDrone.Core/Housekeeping/HousekeepingService.cs @@ -34,7 +34,7 @@ namespace NzbDrone.Core.Housekeeping } catch (Exception ex) { - _logger.Error(ex, "Error running housekeeping task: " + housekeeper.GetType().Name); + _logger.Error(ex, "Error running housekeeping task: {0}", housekeeper.GetType().Name); } } diff --git a/src/NzbDrone.Core/MediaFiles/MovieImport/Aggregation/Aggregators/AggregateQuality.cs b/src/NzbDrone.Core/MediaFiles/MovieImport/Aggregation/Aggregators/AggregateQuality.cs index 298cf191b..a5474ff20 100644 --- a/src/NzbDrone.Core/MediaFiles/MovieImport/Aggregation/Aggregators/AggregateQuality.cs +++ b/src/NzbDrone.Core/MediaFiles/MovieImport/Aggregation/Aggregators/AggregateQuality.cs @@ -63,7 +63,7 @@ namespace NzbDrone.Core.MediaFiles.MovieImport.Aggregation.Aggregators } } - _logger.Trace("Finding quality. Source: {0}. Resolution: {1}", source, resolution); + _logger.Trace("Finding quality. Source: {0}. Resolution: {1}. Modifier {2}", source, resolution, modifier); var quality = new QualityModel(QualityFinder.FindBySourceAndResolution(source, resolution, modifier), revison); diff --git a/src/NzbDrone.Core/Messaging/Commands/CommandExecutor.cs b/src/NzbDrone.Core/Messaging/Commands/CommandExecutor.cs index 4b99dd580..e619122cf 100644 --- a/src/NzbDrone.Core/Messaging/Commands/CommandExecutor.cs +++ b/src/NzbDrone.Core/Messaging/Commands/CommandExecutor.cs @@ -44,13 +44,13 @@ namespace NzbDrone.Core.Messaging.Commands } catch (Exception ex) { - _logger.Error(ex, "Error occurred while executing task " + command.Name); + _logger.Error(ex, "Error occurred while executing task {0}", command.Name); } } } catch (ThreadAbortException ex) { - _logger.Error(ex, "Thread aborted: " + ex.Message); + _logger.Error(ex, "Thread aborted"); Thread.ResetAbort(); } catch (OperationCanceledException) @@ -59,7 +59,7 @@ namespace NzbDrone.Core.Messaging.Commands } catch (Exception ex) { - _logger.Error(ex, "Unknown error in thread: " + ex.Message); + _logger.Error(ex, "Unknown error in thread"); } } diff --git a/src/NzbDrone.Core/Messaging/Commands/CommandQueueManager.cs b/src/NzbDrone.Core/Messaging/Commands/CommandQueueManager.cs index 0d2c4d159..1a199991b 100644 --- a/src/NzbDrone.Core/Messaging/Commands/CommandQueueManager.cs +++ b/src/NzbDrone.Core/Messaging/Commands/CommandQueueManager.cs @@ -1,6 +1,5 @@ using NLog; using NzbDrone.Common; -using NzbDrone.Common.Cache; using NzbDrone.Common.EnsureThat; using NzbDrone.Common.Serializer; using NzbDrone.Core.Lifecycle; diff --git a/src/NzbDrone.Core/Qualities/Quality.cs b/src/NzbDrone.Core/Qualities/Quality.cs index a112e50b6..e417f84f2 100644 --- a/src/NzbDrone.Core/Qualities/Quality.cs +++ b/src/NzbDrone.Core/Qualities/Quality.cs @@ -108,7 +108,7 @@ namespace NzbDrone.Core.Qualities public static Quality BRDISK => new Quality(22, "BR-DISK", Source.BLURAY, 0, Modifier.BRDISK); // new // Others - public static Quality RAWHD => new Quality(10, "Raw-HD", Source.TV, 0, Modifier.RAWHD); + public static Quality RAWHD => new Quality(10, "Raw-HD", Source.TV, 1080, Modifier.RAWHD); static Quality() { diff --git a/src/NzbDrone.Core/Qualities/QualityFinder.cs b/src/NzbDrone.Core/Qualities/QualityFinder.cs index 72f20f2a9..383361810 100644 --- a/src/NzbDrone.Core/Qualities/QualityFinder.cs +++ b/src/NzbDrone.Core/Qualities/QualityFinder.cs @@ -18,7 +18,9 @@ namespace NzbDrone.Core.Qualities return matchingQuality; } - var matchingResolution = Quality.All.Where(q => q.Resolution == resolution) + var matchingModifier = Quality.All.Where(q => q.Modifier == modifer); + + var matchingResolution = matchingModifier.Where(q => q.Resolution == resolution) .OrderBy(q => q.Source) .ToList(); @@ -33,7 +35,7 @@ namespace NzbDrone.Core.Qualities } } - Logger.Warn("Unable to find exact quality for {0} and {1}. Using {2} as fallback", source, resolution, nearestQuality); + Logger.Warn("Unable to find exact quality for {0}, {1}, and {2}. Using {3} as fallback", source, resolution, modifer, nearestQuality); return nearestQuality; } diff --git a/src/NzbDrone.Core/Qualities/QualitySource.cs b/src/NzbDrone.Core/Qualities/QualitySource.cs deleted file mode 100644 index 9610c3d38..000000000 --- a/src/NzbDrone.Core/Qualities/QualitySource.cs +++ /dev/null @@ -1,13 +0,0 @@ -namespace NzbDrone.Core.Qualities -{ - public enum QualitySource - { - Unknown, - Television, - TelevisionRaw, - Web, - WebRip, - DVD, - Bluray - } -}