diff --git a/docs/converter.md b/docs/converter.md index 86dad8427..2fd1ea136 100644 --- a/docs/converter.md +++ b/docs/converter.md @@ -220,7 +220,7 @@ public Task ExcludeConverterSourceTarget() => Verify(new MemoryStream("source-document"u8.ToArray()), "excludesource") .ExcludeTargets("excludesource"); ``` -snippet source | anchor +snippet source | anchor Any existing verified file for an excluded extension is then reported as pending deletion. @@ -243,6 +243,32 @@ public static class ModuleInitializer Excluding every target of a verification is an error, since a verification requires at least one target. +### Avoiding work in a converter + +Building the source document (for example rendering a pdf or a docx) can be expensive. When a target is excluded, it is dropped after the converter has produced it, so the work is wasted. A converter can instead check `IsTargetExcluded` on its `context` and skip producing the target entirely: + + + +```cs +// A converter that builds an expensive source document only when its target is kept. +static ConversionResult ConvertExcludeCheck(string? name, Stream stream, IReadOnlyDictionary context) +{ + List targets = [new("txt", "the rendered pages")]; + if (!context.IsTargetExcluded("excludecheck")) + { + // A real converter would build the document (eg render a pdf) here. + targets.Add(new("excludecheck", new MemoryStream("the source document"u8.ToArray()))); + } + + return new(null, targets); +} +``` +snippet source | anchor + + +`IsTargetExcluded` reflects both the global and the per-verification `ExcludeTargets`, so shipping this check lets a caller opt out of the document build itself, rather than only its snapshot. + + ## Shipping Converters can be shipped as NuGet packages: diff --git a/docs/mdsource/converter.source.md b/docs/mdsource/converter.source.md index 188d12388..0cce35eee 100644 --- a/docs/mdsource/converter.source.md +++ b/docs/mdsource/converter.source.md @@ -87,6 +87,15 @@ snippet: StaticExcludeTargets Excluding every target of a verification is an error, since a verification requires at least one target. +### Avoiding work in a converter + +Building the source document (for example rendering a pdf or a docx) can be expensive. When a target is excluded, it is dropped after the converter has produced it, so the work is wasted. A converter can instead check `IsTargetExcluded` on its `context` and skip producing the target entirely: + +snippet: ConverterExcludeCheck + +`IsTargetExcluded` reflects both the global and the per-verification `ExcludeTargets`, so shipping this check lets a caller opt out of the document build itself, rather than only its snapshot. + + ## Shipping Converters can be shipped as NuGet packages: diff --git a/src/Directory.Build.props b/src/Directory.Build.props index 0c5bb1ec2..e28307389 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -2,7 +2,7 @@ CA1822;CS1591;CS0649;xUnit1026;xUnit1013;CS1573;VerifyTestsProjectDir;VerifySetParameters;PolyFillTargetsForNuget;xUnit1051;NU1608;NU1109 - 31.23.0 + 31.24.0 enable preview 1.0.0 diff --git a/src/StaticSettingsTests/ExcludeTargetsTests.ConverterObservesGlobalExclusion.verified.txt b/src/StaticSettingsTests/ExcludeTargetsTests.ConverterObservesGlobalExclusion.verified.txt new file mode 100644 index 000000000..d77edf693 --- /dev/null +++ b/src/StaticSettingsTests/ExcludeTargetsTests.ConverterObservesGlobalExclusion.verified.txt @@ -0,0 +1 @@ +pages \ No newline at end of file diff --git a/src/StaticSettingsTests/ExcludeTargetsTests.cs b/src/StaticSettingsTests/ExcludeTargetsTests.cs index 730482b46..4284ae87c 100644 --- a/src/StaticSettingsTests/ExcludeTargetsTests.cs +++ b/src/StaticSettingsTests/ExcludeTargetsTests.cs @@ -11,6 +11,26 @@ public async Task GlobalExclusionAppliesToAllVerifications() Assert.Single(result.Files); } + // A converter can see a global exclusion through its context, so it can skip the work. + [Fact] + public async Task ConverterObservesGlobalExclusion() + { + var sawExclusion = false; + VerifierSettings.RegisterStreamConverter( + "globalexcludecheck", + (_, _, context) => + { + sawExclusion = context.IsTargetExcluded("globalexcludecheck"); + return new(null, [new("txt", "pages")]); + }); + VerifierSettings.ExcludeTargets("globalexcludecheck"); + + await Verify(new MemoryStream("input"u8.ToArray()), "globalexcludecheck") + .DisableDiff(); + + Assert.True(sawExclusion); + } + [Fact] public void AfterVerifyHasBeenRunThrows() { diff --git a/src/Verify.Tests/Converters/ExcludeTargetsTests.ConverterKeepsTargetByDefault.verified.excludecheck b/src/Verify.Tests/Converters/ExcludeTargetsTests.ConverterKeepsTargetByDefault.verified.excludecheck new file mode 100644 index 000000000..cccf6de19 --- /dev/null +++ b/src/Verify.Tests/Converters/ExcludeTargetsTests.ConverterKeepsTargetByDefault.verified.excludecheck @@ -0,0 +1 @@ +the source document \ No newline at end of file diff --git a/src/Verify.Tests/Converters/ExcludeTargetsTests.ConverterKeepsTargetByDefault.verified.txt b/src/Verify.Tests/Converters/ExcludeTargetsTests.ConverterKeepsTargetByDefault.verified.txt new file mode 100644 index 000000000..92575dab0 --- /dev/null +++ b/src/Verify.Tests/Converters/ExcludeTargetsTests.ConverterKeepsTargetByDefault.verified.txt @@ -0,0 +1 @@ +the rendered pages \ No newline at end of file diff --git a/src/Verify.Tests/Converters/ExcludeTargetsTests.ConverterSkipsExcludedTarget.verified.txt b/src/Verify.Tests/Converters/ExcludeTargetsTests.ConverterSkipsExcludedTarget.verified.txt new file mode 100644 index 000000000..92575dab0 --- /dev/null +++ b/src/Verify.Tests/Converters/ExcludeTargetsTests.ConverterSkipsExcludedTarget.verified.txt @@ -0,0 +1 @@ +the rendered pages \ No newline at end of file diff --git a/src/Verify.Tests/Converters/ExcludeTargetsTests.cs b/src/Verify.Tests/Converters/ExcludeTargetsTests.cs index fb49a206f..ae4481bb7 100644 --- a/src/Verify.Tests/Converters/ExcludeTargetsTests.cs +++ b/src/Verify.Tests/Converters/ExcludeTargetsTests.cs @@ -14,6 +14,27 @@ public static void Init() => new("txt", "derived from source") ])); + [ModuleInitializer] + public static void ExcludeCheckInit() => + VerifierSettings.RegisterStreamConverter("excludecheck", ConvertExcludeCheck); + + #region ConverterExcludeCheck + + // A converter that builds an expensive source document only when its target is kept. + static ConversionResult ConvertExcludeCheck(string? name, Stream stream, IReadOnlyDictionary context) + { + List targets = [new("txt", "the rendered pages")]; + if (!context.IsTargetExcluded("excludecheck")) + { + // A real converter would build the document (eg render a pdf) here. + targets.Add(new("excludecheck", new MemoryStream("the source document"u8.ToArray()))); + } + + return new(null, targets); + } + + #endregion + class SourceDocument; // The same shape, reached through a file converter rather than a stream converter. @@ -82,6 +103,25 @@ public async Task ExcludeFileConverterSourceTarget() Assert.Single(result.Files); } + // The converter observes the exclusion and never builds the source document. + [Fact] + public async Task ConverterSkipsExcludedTarget() + { + var result = await Verify(new MemoryStream("input"u8.ToArray()), "excludecheck") + .ExcludeTargets("excludecheck") + .DisableDiff(); + Assert.Single(result.Files); + } + + // Without an exclusion the converter builds and keeps the source document. + [Fact] + public async Task ConverterKeepsTargetByDefault() + { + var result = await Verify(new MemoryStream("input"u8.ToArray()), "excludecheck") + .DisableDiff(); + Assert.Equal(2, result.Files.Count()); + } + [Fact] public async Task ExcludingAllTargetsThrows() { diff --git a/src/Verify/Splitters/Settings_ExcludeTargets.cs b/src/Verify/Splitters/Settings_ExcludeTargets.cs index a4bb91b6b..0990adb19 100644 --- a/src/Verify/Splitters/Settings_ExcludeTargets.cs +++ b/src/Verify/Splitters/Settings_ExcludeTargets.cs @@ -13,38 +13,57 @@ public static partial class VerifierSettings public static void ExcludeTargets(params string[] extensions) { InnerVerifier.ThrowIfVerifyHasBeenRun(); - excludedTargets = AddExcludedTargets(excludedTargets, extensions); + excludedTargets = AddExtensions(excludedTargets, extensions); } - internal static bool AnyExcludedTargets(VerifySettings settings) => - excludedTargets != null || - settings.excludedTargets != null; + /// + /// Whether a target with will be excluded from the current + /// verification, via either the global or the per-verification ExcludeTargets. + /// A converter can call this on its context to skip producing an expensive target + /// (eg rendering a document) that would otherwise be dropped. + /// + public static bool IsTargetExcluded(this IReadOnlyDictionary context, string extension) + { + Guards.AgainstBadExtension(extension); + return IsExcluded(context, extension); + } - internal static bool IsTargetExcluded(VerifySettings settings, string extension) => + internal static bool IsExcluded(IReadOnlyDictionary context, string extension) => excludedTargets?.Contains(extension) == true || - settings.excludedTargets?.Contains(extension) == true; + (context.TryGetValue(VerifySettings.excludedTargetsKey, out var value) && + ((HashSet) value).Contains(extension)); - internal static HashSet AddExcludedTargets(HashSet? existing, string[] extensions) + internal static bool AnyExcludedTargets(IReadOnlyDictionary context) => + excludedTargets is { Count: > 0 } || + context.ContainsKey(VerifySettings.excludedTargetsKey); + + // Builds a fresh set rather than mutating existing, so a set shared with cloned settings + // (Context values are copied by reference) is never mutated in place. + internal static HashSet AddExtensions(HashSet? existing, string[] extensions) { if (extensions.Length == 0) { throw new ArgumentException("At least one extension is required.", nameof(extensions)); } - existing ??= new(StringComparer.OrdinalIgnoreCase); + var result = existing == null ? + new(StringComparer.OrdinalIgnoreCase) : + new HashSet(existing, StringComparer.OrdinalIgnoreCase); foreach (var extension in extensions) { Guards.AgainstBadExtension(extension); - existing.Add(extension); + result.Add(extension); } - return existing; + return result; } } public partial class VerifySettings { - internal HashSet? excludedTargets; + // The per-verification excluded set lives in Context so that a converter, which receives + // Context, can consult it via IsTargetExcluded. + internal const string excludedTargetsKey = "Verify.ExcludeTargets"; /// /// Excludes every target with one of from the current verification. @@ -52,8 +71,11 @@ public partial class VerifySettings /// Intended for converters that emit a source document (eg pdf or docx) alongside /// the info file and rendered pages, where committing the source document is not wanted. /// - public void ExcludeTargets(params string[] extensions) => - excludedTargets = VerifierSettings.AddExcludedTargets(excludedTargets, extensions); + public void ExcludeTargets(params string[] extensions) + { + var existing = Context.TryGetValue(excludedTargetsKey, out var value) ? (HashSet) value : null; + Context[excludedTargetsKey] = VerifierSettings.AddExtensions(existing, extensions); + } } public partial class SettingsTask diff --git a/src/Verify/Verifier/InnerVerifier_Inner.cs b/src/Verify/Verifier/InnerVerifier_Inner.cs index eb0c2712c..b3ce46f7e 100644 --- a/src/Verify/Verifier/InnerVerifier_Inner.cs +++ b/src/Verify/Verifier/InnerVerifier_Inner.cs @@ -59,7 +59,7 @@ async Task VerifyInner(object? root, Func? cleanup, IEnumera Func RemoveExcludedTargets(List targets, Func cleanup, out bool removed) { removed = false; - if (!VerifierSettings.AnyExcludedTargets(settings)) + if (!VerifierSettings.AnyExcludedTargets(settings.Context)) { return cleanup; } @@ -67,7 +67,7 @@ Func RemoveExcludedTargets(List targets, Func cleanup, out b for (var index = targets.Count - 1; index >= 0; index--) { var target = targets[index]; - if (!VerifierSettings.IsTargetExcluded(settings, target.Extension)) + if (!VerifierSettings.IsExcluded(settings.Context, target.Extension)) { continue; } diff --git a/src/Verify/VerifySettings.cs b/src/Verify/VerifySettings.cs index 19c8f964f..e20f60181 100644 --- a/src/Verify/VerifySettings.cs +++ b/src/Verify/VerifySettings.cs @@ -44,11 +44,7 @@ public VerifySettings(VerifySettings? settings) // (AppendFile/UseStreamComparer/etc.) would mutate a shared base settings // instance and leak across tests. appendedFiles = settings.appendedFiles?.ToList(); - if (settings.excludedTargets != null) - { - excludedTargets = new(settings.excludedTargets, StringComparer.OrdinalIgnoreCase); - } - + // excludedTargets lives in Context (see ExcludeTargets), so it is cloned by the Context copy below UniqueDirectory = settings.UniqueDirectory; Directory = settings.Directory; autoVerify = settings.autoVerify;