diff --git a/claude.md b/claude.md index bdb8bef564..c4982d373f 100644 --- a/claude.md +++ b/claude.md @@ -132,6 +132,13 @@ dotnet tool restore --tool-manifest src/.config/dotnet-tools.json **Builder Pattern**: `SettingsTask` provides fluent API with ~50 configuration methods that are lazily evaluated at verification time. +**Source and derived targets**: A converter can say which of its targets is the document and which were computed from it (`ConversionResult(info, source, derived)`, or the `PagedConversion` builder for documents with pages, both in `src/Verify/Splitters/`): +- `InnerVerifier.Adopt` (`InnerVerifier_Stream.cs`) is the only place a `ConversionToken` is created. It stamps the targets of a conversion, names them relative to the target that was converted, and marks the source, which is never converted again. +- `VerifyEngine.HandleResults` decides every file name first (`Plan`), compares sources ahead of the rest (`CompareOrder`, so a differing source makes its derived targets bypass their comparers), and hands results on in planned order, which is the order callbacks and the exception message have always used. +- `VerifyEngine.Report` runs once, after the delete/new/not-equal callbacks, and is the only caller of DiffEngine for pending files: deletes that stand alone, moves that stand alone (sources among them), moves derived from a pending source (`DiffRunner.LaunchDerived*`), then deletes derived from one. Test seams: `VerifyEngine.LaunchDiff` and `RaisedDeletes.AddDelete`. +- The settings a paged converter reads (`PageText`, `PagesToInclude`, `ExcludeDerivedTargets`) follow the `ExcludeTargets` pattern: a static on `VerifierSettings`, a `Context` key on `VerifySettings`, a `SettingsTask` wrapper, and a reader on the converter's `context`. +- User and plugin-author docs: `docs/paged-documents.md` and the "Source and derived targets" section of `docs/converter.md`. + **Counter Pattern**: Deduplicates repeated values in filenames: - First occurrence: `Date`, second: `Date_1`, third: `Date_2`, etc. - Separate counters for DateTime, DateTimeOffset, Guid, etc. diff --git a/docs/comparer.md b/docs/comparer.md index df8eff8339..ca7cd33a74 100644 --- a/docs/comparer.md +++ b/docs/comparer.md @@ -205,6 +205,8 @@ public static ConversionResult ConvertDocument(Stream document, IReadOnlyDiction The flag must be set on the source target, and that target must precede the derived targets in the conversion result. +A converter that says which of its targets is the source, and which were derived from it, needs no flag. Verify compares the source first, and when it differs compares its derived targets exactly. That applies only to the targets derived from that source, where the flag applies to every target after it. See [Source and derived targets](/docs/converter.md#source-and-derived-targets). + ## Default Comparison diff --git a/docs/context.md b/docs/context.md index 5c35676554..5392464203 100644 --- a/docs/context.md +++ b/docs/context.md @@ -90,7 +90,7 @@ Values that are the same for every test do not need Context. A static field is s ## Reserved keys -Verify uses the same dictionary for some per-verification state, under keys prefixed with `Verify.`. For example `ExcludeTargets` stores its extensions under `Verify.ExcludeTargets`, which is what allows a converter to call `context.IsTargetExcluded("png")`. Keys prefixed with `Verify.` should be treated as reserved. +Verify uses the same dictionary for some per-verification state, under keys prefixed with `Verify.`. For example `ExcludeTargets` stores its extensions under `Verify.ExcludeTargets`, which is what allows a converter to call `context.IsTargetExcluded("png")`. The settings for [paged documents](/docs/paged-documents.md) are read the same way: `context.PageTextPlacement()`, `context.IsPageIncluded(1)` and `context.IsDerivedTargetExcluded("png")`. Keys prefixed with `Verify.` should be treated as reserved. ## Copy behavior diff --git a/docs/converter.md b/docs/converter.md index b5bd30ea5b..9f7b9cf609 100644 --- a/docs/converter.md +++ b/docs/converter.md @@ -11,12 +11,14 @@ Converters are used to split a target into its component parts, then verify each When a target is split the result is: - * An info file (containing the metadata of the target) serialized as json. File name: `{TestType}.{TestMethod}.info.verified.txt` - * Zero or more documents of a specified extension. File name: `{TestType}.{TestMethod}.{Index}.verified.{Extension}` + * An info file (containing the metadata of the target) serialized as json. File name: `{TestType}.{TestMethod}.verified.txt` + * Zero or more targets of a specified extension. File name: `{TestType}.{TestMethod}#{Name}.verified.{Extension}` for a target the converter named. Targets with no name that share an extension are told apart by an index: `{TestType}.{TestMethod}#00.verified.{Extension}`. Converters are registered globally. The `context` parameter passed to a conversion carries per-test information. See [Context](/docs/context.md). +A converter for a document with pages is best built on `PagedConversion`. See [Paged documents](/docs/paged-documents.md). + ## Usage scenarios @@ -229,6 +231,58 @@ return new( +## Source and derived targets + +Many converters return the document they were given, alongside what they computed from it: a csv for each sheet of a workbook, an image of each page of a pdf. A converter can say which is which, by passing the document as the `source` and the rest as `derived`: + + + +```cs +// A converter of a workbook: the workbook is the source, and a csv of each sheet is derived +static ConversionResult ConvertWorkbook(string? name, Stream stream, IReadOnlyDictionary context) +{ + var workbook = Workbook.Load(stream); + + var sheets = new List(); + foreach (var sheet in workbook.Sheets) + { + // Named by what it is. The name of the target being converted is added by Verify + sheets.Add(new("csv", sheet.ToCsv(), sheet.Name)); + } + + Target? source = null; + if (!context.IsTargetExcluded("xlsx")) + { + source = new("xlsx", workbook.Save()); + } + + return new( + info: new + { + workbook.Author + }, + source, + derived: sheets); +} +``` +snippet source | anchor + + +Verify then does the following, so that no converter has to: + + * **Names.** The targets are named relative to the target that was converted, so the `name` the converter is passed is not used. A sheet named `Sheet1` becomes `{TestType}.{TestMethod}#Sheet1.verified.csv`. Where the workbook is itself a target named `Attachment1`, the sheet becomes `#Attachment1.Sheet1` and the workbook takes `#Attachment1`. So does its info file when the workbook is a target passed to the verification. For a workbook found inside another converted document, the info is gathered into that document's info file. + * **No second conversion.** The source is not converted again, whatever its extension. A converter registered for both `xls` and `xlsx` can return an `xls` it was given as an `xlsx`. + * **Comparison.** The source is compared first. When it differs, its derived targets skip their registered [comparers](/docs/comparer.md#bypass-comparers-for-derived-targets) and are compared exactly. Only its own: a second document of the same verification is not affected. + * **Review.** The diff tool is told the derived files came from the source. [DiffEngineViewer](https://github.com/VerifyTests/DiffEngine/blob/main/docs/viewer.md#files-derived-from-a-document) shows a document it can draw as one row, with what was derived from it beneath, and accepts them together. Other diff tools are given each file as before. Verified files that the conversion no longer produces, such as a page a document has lost, are deleted along with the accept of the document. + * **Exclusion.** `ExcludeDerivedTargets` applies to the derived targets and to nothing else. See [Leaving out what was derived](/docs/paged-documents.md#leaving-out-what-was-derived). + +The info file counts as derived when it holds only what converters returned. With an `info` argument passed to the verification, or a [JsonAppender](/docs/jsonappender.md) in play, it holds something of the test's as well and stands alone. + +`source` is null where the document is not wanted as a target. The derived targets then stand alone as well, and are still named the same way. + +A target that is not the source can opt out of further conversion with `performConversion: false`. + + ## Excluding targets Some converters emit the source document (for example a `pdf`, `docx`, or `xlsx`) alongside the info file and the derived targets. That source document is then committed as a `.verified.{extension}` file. Where the document is large, or where its bytes cannot be made deterministic, it can be excluded from the snapshot. The info file and the derived targets continue to verify. @@ -291,6 +345,8 @@ static ConversionResult ConvertExcludeCheck(string? name, Stream stream, IReadOn `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. +`IsDerivedTargetExcluded` is the same check for a [derived target](#source-and-derived-targets). It reflects `ExcludeDerivedTargets` as well as `ExcludeTargets`. + ## Shipping diff --git a/docs/inline-snapshots.md b/docs/inline-snapshots.md index fba3b2d4c8..5b5d4e9c9f 100644 --- a/docs/inline-snapshots.md +++ b/docs/inline-snapshots.md @@ -230,6 +230,8 @@ new Target("md", page1) The whole verification then falls back to files. +The info file of a converter that [names its source](/docs/converter.md#source-and-derived-targets) is opted out the same way by Verify. It is a file of the document, reviewed and accepted together with the document and the files derived from it. + ## Calling Verify through a wrapper diff --git a/docs/mdsource/comparer.source.md b/docs/mdsource/comparer.source.md index f4df8b3458..08f162b36a 100644 --- a/docs/mdsource/comparer.source.md +++ b/docs/mdsource/comparer.source.md @@ -82,6 +82,8 @@ snippet: BypassComparersForSubsequentOnDifference The flag must be set on the source target, and that target must precede the derived targets in the conversion result. +A converter that says which of its targets is the source, and which were derived from it, needs no flag. Verify compares the source first, and when it differs compares its derived targets exactly. That applies only to the targets derived from that source, where the flag applies to every target after it. See [Source and derived targets](/docs/converter.md#source-and-derived-targets). + ## Default Comparison diff --git a/docs/mdsource/context.source.md b/docs/mdsource/context.source.md index 4645228559..bd4135946a 100644 --- a/docs/mdsource/context.source.md +++ b/docs/mdsource/context.source.md @@ -28,7 +28,7 @@ Values that are the same for every test do not need Context. A static field is s ## Reserved keys -Verify uses the same dictionary for some per-verification state, under keys prefixed with `Verify.`. For example `ExcludeTargets` stores its extensions under `Verify.ExcludeTargets`, which is what allows a converter to call `context.IsTargetExcluded("png")`. Keys prefixed with `Verify.` should be treated as reserved. +Verify uses the same dictionary for some per-verification state, under keys prefixed with `Verify.`. For example `ExcludeTargets` stores its extensions under `Verify.ExcludeTargets`, which is what allows a converter to call `context.IsTargetExcluded("png")`. The settings for [paged documents](/docs/paged-documents.md) are read the same way: `context.PageTextPlacement()`, `context.IsPageIncluded(1)` and `context.IsDerivedTargetExcluded("png")`. Keys prefixed with `Verify.` should be treated as reserved. ## Copy behavior diff --git a/docs/mdsource/converter.source.md b/docs/mdsource/converter.source.md index d0f402a8fb..a009ff3ff1 100644 --- a/docs/mdsource/converter.source.md +++ b/docs/mdsource/converter.source.md @@ -4,12 +4,14 @@ Converters are used to split a target into its component parts, then verify each When a target is split the result is: - * An info file (containing the metadata of the target) serialized as json. File name: `{TestType}.{TestMethod}.info.verified.txt` - * Zero or more documents of a specified extension. File name: `{TestType}.{TestMethod}.{Index}.verified.{Extension}` + * An info file (containing the metadata of the target) serialized as json. File name: `{TestType}.{TestMethod}.verified.txt` + * Zero or more targets of a specified extension. File name: `{TestType}.{TestMethod}#{Name}.verified.{Extension}` for a target the converter named. Targets with no name that share an extension are told apart by an index: `{TestType}.{TestMethod}#00.verified.{Extension}`. Converters are registered globally. The `context` parameter passed to a conversion carries per-test information. See [Context](/docs/context.md). +A converter for a document with pages is best built on `PagedConversion`. See [Paged documents](/docs/paged-documents.md). + ## Usage scenarios @@ -77,6 +79,27 @@ If cleanup needs to occur after verification a callback can be passes to `Conver snippet: ConversionResultWithCleanup +## Source and derived targets + +Many converters return the document they were given, alongside what they computed from it: a csv for each sheet of a workbook, an image of each page of a pdf. A converter can say which is which, by passing the document as the `source` and the rest as `derived`: + +snippet: SourceAndDerivedTargets + +Verify then does the following, so that no converter has to: + + * **Names.** The targets are named relative to the target that was converted, so the `name` the converter is passed is not used. A sheet named `Sheet1` becomes `{TestType}.{TestMethod}#Sheet1.verified.csv`. Where the workbook is itself a target named `Attachment1`, the sheet becomes `#Attachment1.Sheet1` and the workbook takes `#Attachment1`. So does its info file when the workbook is a target passed to the verification. For a workbook found inside another converted document, the info is gathered into that document's info file. + * **No second conversion.** The source is not converted again, whatever its extension. A converter registered for both `xls` and `xlsx` can return an `xls` it was given as an `xlsx`. + * **Comparison.** The source is compared first. When it differs, its derived targets skip their registered [comparers](/docs/comparer.md#bypass-comparers-for-derived-targets) and are compared exactly. Only its own: a second document of the same verification is not affected. + * **Review.** The diff tool is told the derived files came from the source. [DiffEngineViewer](https://github.com/VerifyTests/DiffEngine/blob/main/docs/viewer.md#files-derived-from-a-document) shows a document it can draw as one row, with what was derived from it beneath, and accepts them together. Other diff tools are given each file as before. Verified files that the conversion no longer produces, such as a page a document has lost, are deleted along with the accept of the document. + * **Exclusion.** `ExcludeDerivedTargets` applies to the derived targets and to nothing else. See [Leaving out what was derived](/docs/paged-documents.md#leaving-out-what-was-derived). + +The info file counts as derived when it holds only what converters returned. With an `info` argument passed to the verification, or a [JsonAppender](/docs/jsonappender.md) in play, it holds something of the test's as well and stands alone. + +`source` is null where the document is not wanted as a target. The derived targets then stand alone as well, and are still named the same way. + +A target that is not the source can opt out of further conversion with `performConversion: false`. + + ## Excluding targets Some converters emit the source document (for example a `pdf`, `docx`, or `xlsx`) alongside the info file and the derived targets. That source document is then committed as a `.verified.{extension}` file. Where the document is large, or where its bytes cannot be made deterministic, it can be excluded from the snapshot. The info file and the derived targets continue to verify. @@ -102,6 +125,8 @@ 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. +`IsDerivedTargetExcluded` is the same check for a [derived target](#source-and-derived-targets). It reflects `ExcludeDerivedTargets` as well as `ExcludeTargets`. + ## Shipping diff --git a/docs/mdsource/doc-index.include.md b/docs/mdsource/doc-index.include.md index 6f258f3838..ec231d661c 100644 --- a/docs/mdsource/doc-index.include.md +++ b/docs/mdsource/doc-index.include.md @@ -42,6 +42,7 @@ * [Kill process locking file](/docs/kill-process-locking-file.md) * [Comparers](/docs/comparer.md) * [Converters](/docs/converter.md) + * [Paged documents](/docs/paged-documents.md) * [Context](/docs/context.md) * [Recording](/docs/recording.md) * [Explicit Targets](/docs/explicit-targets.md) diff --git a/docs/mdsource/inline-snapshots.source.md b/docs/mdsource/inline-snapshots.source.md index acb0d5b6ea..b2036008bb 100644 --- a/docs/mdsource/inline-snapshots.source.md +++ b/docs/mdsource/inline-snapshots.source.md @@ -124,6 +124,8 @@ new Target("md", page1) The whole verification then falls back to files. +The info file of a converter that [names its source](/docs/converter.md#source-and-derived-targets) is opted out the same way by Verify. It is a file of the document, reviewed and accepted together with the document and the files derived from it. + ## Calling Verify through a wrapper diff --git a/docs/mdsource/naming.source.md b/docs/mdsource/naming.source.md index fb3668e239..850d26c0c2 100644 --- a/docs/mdsource/naming.source.md +++ b/docs/mdsource/naming.source.md @@ -376,6 +376,25 @@ if (maps.TryGetVerified(receivedPath, out var verifiedPath)) `ReceivedMaps.Pairs` enumerates every pair instead, for accepting a whole run at once. +A file that a [converter](converter.md#source-and-derived-targets) derived from a document, such as the image of a page, has a third line while that document is itself pending: the received path of the document. + +``` +C:\code\MyProject\Tests\TheTest.TheMethod#page_0001.DotNet11_0.received.png +C:\code\MyProject\Tests\TheTest.TheMethod#page_0001.verified.png +C:\code\MyProject\Tests\TheTest.TheMethod.DotNet11_0.received.pdf +``` + +`ReceivedMaps.TryGetSource` reads it, so that a tool can present a document and what was derived from it as one change, and accept them together: + +```cs +if (maps.TryGetSource(receivedPath, out var documentReceivedPath)) +{ + // receivedPath was derived from the document at documentReceivedPath +} +``` + +It answers false for a file that stands alone: one that was not derived from a document, the document itself, and a file whose document has since been accepted. + It scans the directory recursively, so it can be pointed at a project or a repository root. `.git` and `node_modules` are skipped. Notes: diff --git a/docs/mdsource/paged-documents.source.md b/docs/mdsource/paged-documents.source.md new file mode 100644 index 0000000000..f85a903c1f --- /dev/null +++ b/docs/mdsource/paged-documents.source.md @@ -0,0 +1,130 @@ +# Paged documents + +A [converter](/docs/converter.md) for a document with pages (a pdf, a Word document, a presentation, a multi frame image) verifies more than the document. It also verifies what it computed from the document: an image of each page, the text of each page, and an info file describing the whole. + +How those files are named, where the text goes, and which pages are verified are decided by Verify rather than by each converter, so every converter built on `PagedConversion` behaves the same way and is controlled by the same settings. + + +## The files + +For a test `Tests.Report` verifying a pdf: + +| File | Holds | +| --- | --- | +| `Tests.Report.verified.pdf` | The document | +| `Tests.Report.verified.txt` | The info file: what the converter says of the document, the page count, and the text | +| `Tests.Report#page_0001.verified.png` | The first page, drawn | +| `Tests.Report#page_0002.verified.png` | The second page, drawn | + +Page numbers are 1 based, and a page keeps its number when other pages are left out. + +The info file has one shape, whichever converter wrote it: + +snippet: PagedConversionTests.Sample.verified.txt + +`Document` is whatever the converter says of the document as a whole, and each page's `Info` whatever it says of that page. A member with nothing in it is left out, and when there is nothing to say at all no info file is written. + + +## Where the text goes + +By default the text read from a document is in the info file, under each page. `PageText` moves it: + +| `PageTextPlacement` | Text | +| --- | --- | +| `InInfo` | In the info file. The default. | +| `PerPage` | In a file per page, named as the image of the page is: `Tests.Report#page_0001.verified.txt`. | +| `None` | Not verified. A converter can skip reading it. | + +snippet: PageTextPerPage + +Some converters cannot say which page a part of the text is on, and read the document as one text. That goes where the text of a page would have gone: `Text` in the info file, or `Tests.Report#text.verified.txt` under `PerPage`. + + +## Which pages are verified + +`PagesToInclude` limits what is verified to the first pages of a document: + +snippet: PagesToIncludeCount + +Or to the pages a delegate accepts: + +snippet: PagesToIncludeDelegate + +The document itself is still verified whole, and `PageCount` in the info file is still the number of pages it has. + + +## Leaving out what was derived + +`ExcludeDerivedTargets` drops the derived targets with an extension, for example the page images, where only the document and its text are wanted: + +snippet: ExcludeDerivedTargets + +It applies only to what a converter derived from a document. [ExcludeTargets](/docs/converter.md#excluding-targets) applies to every target with the extension, so `ExcludeTargets("png")` would also exclude a png that is itself the thing being verified. It remains the way to leave out the document: `ExcludeTargets("pdf")`. + + +## For every test + +All three can be set once, at initialization: + +snippet: StaticPagedDocuments + +A setting on a verification is used in place of the global one, and its exclusions are added to the global ones. + + +## Reviewing in a diff tool + +A change to a three page pdf is a change to at least five files. Verify tells [DiffEngine](https://github.com/VerifyTests/DiffEngine) which of them were derived from the document, and [DiffEngineViewer](https://github.com/VerifyTests/DiffEngine/blob/main/docs/viewer.md#files-derived-from-a-document), which draws the document's pages itself, shows them as one row and accepts them as one. Any other diff tool is given each file as before. + +The same is recorded for tooling that runs after the tests: see the [received map file](/docs/naming.md#received-map-file). + +When the document has changed, the files derived from it are compared exactly, skipping any [comparer](/docs/comparer.md) registered for them. A comparer exists to tolerate differences, and a document that has changed is the one case where its pages should not be given the benefit of the doubt. + +The info file of a document is never an [inline snapshot](/docs/inline-snapshots.md) under the global switch. It is a file of the document, reviewed and accepted with the rest. + + +## Writing a converter + +`PagedConversion` builds the result of a converter from the document and its pages: + +snippet: PagedConversion + +snippet: PagedConversionVerify + + * `Source` is the document. It is left out when [IsTargetExcluded](/docs/converter.md#avoiding-work-in-a-converter) says its extension is excluded. + * `Pages` gives the numbers of the pages to verify and records the page count. `IsPageIncluded` answers for one page. + * `IncludeImages` and `IncludeText` say whether a page's image and text will be kept, so that neither is produced only to be dropped. + * `AddPage` takes whichever of an image, the text and an info a page has. Empty text is no text. An info with nothing to say is best passed as null, so that the page has no empty `Info` member. + * `AddImages` is for a renderer that draws every page at once and so cannot skip one. The images of pages that are not verified are dropped. + * `Text` is for a converter that reads the document as one text. + * `AddDerived` adds any other target computed from the document, such as a csv of a sheet. It has to be named, even when it is the only one, so that a second sheet adds a file rather than renaming the first. + * `ImageExtension` and `TextExtension` change `png` and `txt`, for example to `md` for a converter that reads a document as markdown. + +The `name` a converter is passed is not used. Verify names what a converter returns relative to the target that was converted, so a document that is the attachment `Attachment1` of some other target becomes: + +``` +Tests.Mail#Attachment1.verified.pdf +Tests.Mail#Attachment1.page_0001.verified.png +``` + +Its info is gathered into the info file of the target it was found in. A document passed to the verification as a target named `Attachment1` has an info file of its own, `Tests.Mail#Attachment1.verified.txt`. + +A converter with no pages, for example a workbook split into a csv per sheet, uses the constructor of `ConversionResult` that `PagedConversion` is built on. See [Source and derived targets](/docs/converter.md#source-and-derived-targets). + + +## Migrating from 33 + +Up to version 33 each converter chose its own file names and settings. Converters that have moved to `PagedConversion` differ in these ways: + + * Page files are named `#page_0001`, not by an index: `#00`, `#01`. The index was 0 based, was missing altogether for a document with one page, and for a text file was one higher than for its image. + * The text of a page is in the info file unless `PageText` says otherwise. + * The info file has the one shape above. + * A sheet is always named. Converters that left the csv of a one sheet workbook unnamed, as `.verified.csv`, now write `#Sheet1.verified.csv`. + * `PagesToInclude`, `PageText` and `ExcludeDerivedTargets` are members of `VerifySettings`, `SettingsTask` and `VerifierSettings`. The methods and enums a converter had for the same purpose are gone: an `ExcludeDerivedTargets("png")` where there was an option to verify only the text. + +Renamed snapshots show as a new file and a pending delete. Accepting both, or running once with [AutoVerify](/docs/autoverify.md), moves a test over. Where rendering is byte for byte deterministic the content of a page file is unchanged, and source control shows it as a rename. + +Where page images are compared with a tolerance, an accepted page is a fresh render, and can differ in its bytes from the one committed under the old name. Renaming the committed files to the new names instead keeps them as they were. The pages are then compared by their comparer as before, while the document itself is unchanged, and only the info file is left to accept. + +A converter has to move with Verify. One built against 33 still compiles against 34, but its own `PagesToInclude` extension methods are hidden by the members of the same name that Verify now has, so its page filter is never set. + +For code calling `InnerVerifier(directory, name)` directly: a target with a name is now written to `{name}#{TargetName}.verified.{extension}`. It was written to `{name}.verified.{extension}`, which every named target of a verification shared. diff --git a/docs/naming.md b/docs/naming.md index f268acd8bd..c2d39ed0ea 100644 --- a/docs/naming.md +++ b/docs/naming.md @@ -951,6 +951,25 @@ if (maps.TryGetVerified(receivedPath, out var verifiedPath)) `ReceivedMaps.Pairs` enumerates every pair instead, for accepting a whole run at once. +A file that a [converter](converter.md#source-and-derived-targets) derived from a document, such as the image of a page, has a third line while that document is itself pending: the received path of the document. + +``` +C:\code\MyProject\Tests\TheTest.TheMethod#page_0001.DotNet11_0.received.png +C:\code\MyProject\Tests\TheTest.TheMethod#page_0001.verified.png +C:\code\MyProject\Tests\TheTest.TheMethod.DotNet11_0.received.pdf +``` + +`ReceivedMaps.TryGetSource` reads it, so that a tool can present a document and what was derived from it as one change, and accept them together: + +```cs +if (maps.TryGetSource(receivedPath, out var documentReceivedPath)) +{ + // receivedPath was derived from the document at documentReceivedPath +} +``` + +It answers false for a file that stands alone: one that was not derived from a document, the document itself, and a file whose document has since been accepted. + It scans the directory recursively, so it can be pointed at a project or a repository root. `.git` and `node_modules` are skipped. Notes: diff --git a/docs/paged-documents.md b/docs/paged-documents.md new file mode 100644 index 0000000000..982c7f135c --- /dev/null +++ b/docs/paged-documents.md @@ -0,0 +1,255 @@ + + +# Paged documents + +A [converter](/docs/converter.md) for a document with pages (a pdf, a Word document, a presentation, a multi frame image) verifies more than the document. It also verifies what it computed from the document: an image of each page, the text of each page, and an info file describing the whole. + +How those files are named, where the text goes, and which pages are verified are decided by Verify rather than by each converter, so every converter built on `PagedConversion` behaves the same way and is controlled by the same settings. + + +## The files + +For a test `Tests.Report` verifying a pdf: + +| File | Holds | +| --- | --- | +| `Tests.Report.verified.pdf` | The document | +| `Tests.Report.verified.txt` | The info file: what the converter says of the document, the page count, and the text | +| `Tests.Report#page_0001.verified.png` | The first page, drawn | +| `Tests.Report#page_0002.verified.png` | The second page, drawn | + +Page numbers are 1 based, and a page keeps its number when other pages are left out. + +The info file has one shape, whichever converter wrote it: + + + +```txt +{ + Document: { + Title: The title + }, + PageCount: 2, + Pages: [ + { + Number: 1, + Text: The first page + }, + { + Number: 2, + Text: The second page + } + ] +} +``` +snippet source | anchor + + +`Document` is whatever the converter says of the document as a whole, and each page's `Info` whatever it says of that page. A member with nothing in it is left out, and when there is nothing to say at all no info file is written. + + +## Where the text goes + +By default the text read from a document is in the info file, under each page. `PageText` moves it: + +| `PageTextPlacement` | Text | +| --- | --- | +| `InInfo` | In the info file. The default. | +| `PerPage` | In a file per page, named as the image of the page is: `Tests.Report#page_0001.verified.txt`. | +| `None` | Not verified. A converter can skip reading it. | + + + +```cs +Verify(pdf, "pdf") + .PageText(PageTextPlacement.PerPage); +``` +snippet source | anchor + + +Some converters cannot say which page a part of the text is on, and read the document as one text. That goes where the text of a page would have gone: `Text` in the info file, or `Tests.Report#text.verified.txt` under `PerPage`. + + +## Which pages are verified + +`PagesToInclude` limits what is verified to the first pages of a document: + + + +```cs +Verify(pdf, "pdf") + .PagesToInclude(2); +``` +snippet source | anchor + + +Or to the pages a delegate accepts: + + + +```cs +Verify(pdf, "pdf") + .PagesToInclude(pageNumber => pageNumber is 1 or 5); +``` +snippet source | anchor + + +The document itself is still verified whole, and `PageCount` in the info file is still the number of pages it has. + + +## Leaving out what was derived + +`ExcludeDerivedTargets` drops the derived targets with an extension, for example the page images, where only the document and its text are wanted: + + + +```cs +Verify(pdf, "pdf") + .ExcludeDerivedTargets("png"); +``` +snippet source | anchor + + +It applies only to what a converter derived from a document. [ExcludeTargets](/docs/converter.md#excluding-targets) applies to every target with the extension, so `ExcludeTargets("png")` would also exclude a png that is itself the thing being verified. It remains the way to leave out the document: `ExcludeTargets("pdf")`. + + +## For every test + +All three can be set once, at initialization: + + + +```cs +public static class ModuleInitializer +{ + [ModuleInitializer] + public static void Init() + { + VerifierSettings.PageText(PageTextPlacement.PerPage); + VerifierSettings.PagesToInclude(10); + VerifierSettings.ExcludeDerivedTargets("png"); + } +} +``` +snippet source | anchor + + +A setting on a verification is used in place of the global one, and its exclusions are added to the global ones. + + +## Reviewing in a diff tool + +A change to a three page pdf is a change to at least five files. Verify tells [DiffEngine](https://github.com/VerifyTests/DiffEngine) which of them were derived from the document, and [DiffEngineViewer](https://github.com/VerifyTests/DiffEngine/blob/main/docs/viewer.md#files-derived-from-a-document), which draws the document's pages itself, shows them as one row and accepts them as one. Any other diff tool is given each file as before. + +The same is recorded for tooling that runs after the tests: see the [received map file](/docs/naming.md#received-map-file). + +When the document has changed, the files derived from it are compared exactly, skipping any [comparer](/docs/comparer.md) registered for them. A comparer exists to tolerate differences, and a document that has changed is the one case where its pages should not be given the benefit of the doubt. + +The info file of a document is never an [inline snapshot](/docs/inline-snapshots.md) under the global switch. It is a file of the document, reviewed and accepted with the rest. + + +## Writing a converter + +`PagedConversion` builds the result of a converter from the document and its pages: + + + +```cs +// A document that is a title, then the text of each page, with a | between them +static ConversionResult ConvertSlides(string? name, Stream stream, IReadOnlyDictionary context) +{ + var bytes = ReadBytes(stream); + var lines = Encoding.UTF8.GetString(bytes).Split('|'); + + var conversion = new PagedConversion(context) + { + Info = new + { + Title = lines[0] + } + }; + + // The document itself, unless ExcludeTargets says it is not wanted + if (!context.IsTargetExcluded("slides")) + { + conversion.Source(new("slides", new MemoryStream(bytes))); + } + + // The pages PagesToInclude asks for, which is all of them by default + foreach (var number in conversion.Pages(pageCount: lines.Length - 1)) + { + // Rendering and reading are the expensive parts, so ask before doing either + Stream? image = null; + if (conversion.IncludeImages) + { + image = Render(number); + } + + string? text = null; + if (conversion.IncludeText) + { + text = ReadText(lines[number]); + } + + conversion.AddPage(number, image, text); + } + + return conversion.Build(); +} +``` +snippet source | anchor + + + + +```cs +[Fact] +public Task Sample() => + Verify(Slides(), "slides"); +``` +snippet source | anchor + + + * `Source` is the document. It is left out when [IsTargetExcluded](/docs/converter.md#avoiding-work-in-a-converter) says its extension is excluded. + * `Pages` gives the numbers of the pages to verify and records the page count. `IsPageIncluded` answers for one page. + * `IncludeImages` and `IncludeText` say whether a page's image and text will be kept, so that neither is produced only to be dropped. + * `AddPage` takes whichever of an image, the text and an info a page has. Empty text is no text. An info with nothing to say is best passed as null, so that the page has no empty `Info` member. + * `AddImages` is for a renderer that draws every page at once and so cannot skip one. The images of pages that are not verified are dropped. + * `Text` is for a converter that reads the document as one text. + * `AddDerived` adds any other target computed from the document, such as a csv of a sheet. It has to be named, even when it is the only one, so that a second sheet adds a file rather than renaming the first. + * `ImageExtension` and `TextExtension` change `png` and `txt`, for example to `md` for a converter that reads a document as markdown. + +The `name` a converter is passed is not used. Verify names what a converter returns relative to the target that was converted, so a document that is the attachment `Attachment1` of some other target becomes: + +``` +Tests.Mail#Attachment1.verified.pdf +Tests.Mail#Attachment1.page_0001.verified.png +``` + +Its info is gathered into the info file of the target it was found in. A document passed to the verification as a target named `Attachment1` has an info file of its own, `Tests.Mail#Attachment1.verified.txt`. + +A converter with no pages, for example a workbook split into a csv per sheet, uses the constructor of `ConversionResult` that `PagedConversion` is built on. See [Source and derived targets](/docs/converter.md#source-and-derived-targets). + + +## Migrating from 33 + +Up to version 33 each converter chose its own file names and settings. Converters that have moved to `PagedConversion` differ in these ways: + + * Page files are named `#page_0001`, not by an index: `#00`, `#01`. The index was 0 based, was missing altogether for a document with one page, and for a text file was one higher than for its image. + * The text of a page is in the info file unless `PageText` says otherwise. + * The info file has the one shape above. + * A sheet is always named. Converters that left the csv of a one sheet workbook unnamed, as `.verified.csv`, now write `#Sheet1.verified.csv`. + * `PagesToInclude`, `PageText` and `ExcludeDerivedTargets` are members of `VerifySettings`, `SettingsTask` and `VerifierSettings`. The methods and enums a converter had for the same purpose are gone: an `ExcludeDerivedTargets("png")` where there was an option to verify only the text. + +Renamed snapshots show as a new file and a pending delete. Accepting both, or running once with [AutoVerify](/docs/autoverify.md), moves a test over. Where rendering is byte for byte deterministic the content of a page file is unchanged, and source control shows it as a rename. + +Where page images are compared with a tolerance, an accepted page is a fresh render, and can differ in its bytes from the one committed under the old name. Renaming the committed files to the new names instead keeps them as they were. The pages are then compared by their comparer as before, while the document itself is unchanged, and only the info file is left to accept. + +A converter has to move with Verify. One built against 33 still compiles against 34, but its own `PagesToInclude` extension methods are hidden by the members of the same name that Verify now has, so its page filter is never set. + +For code calling `InnerVerifier(directory, name)` directly: a target with a name is now written to `{name}#{TargetName}.verified.{extension}`. It was written to `{name}.verified.{extension}`, which every named target of a verification shared. diff --git a/docs/readme.md b/docs/readme.md index ab6b7633e7..b2f44bfd2f 100644 --- a/docs/readme.md +++ b/docs/readme.md @@ -51,6 +51,7 @@ To change this file edit the source file and then run MarkdownSnippets. * [Kill process locking file](/docs/kill-process-locking-file.md) * [Comparers](/docs/comparer.md) * [Converters](/docs/converter.md) + * [Paged documents](/docs/paged-documents.md) * [Context](/docs/context.md) * [Recording](/docs/recording.md) * [Explicit Targets](/docs/explicit-targets.md) diff --git a/readme.md b/readme.md index 0a2c8c267c..da2d91bb6f 100644 --- a/readme.md +++ b/readme.md @@ -1154,6 +1154,7 @@ Browser testing via * [Kill process locking file](/docs/kill-process-locking-file.md) * [Comparers](/docs/comparer.md) * [Converters](/docs/converter.md) + * [Paged documents](/docs/paged-documents.md) * [Context](/docs/context.md) * [Recording](/docs/recording.md) * [Explicit Targets](/docs/explicit-targets.md) diff --git a/src/Directory.Build.props b/src/Directory.Build.props index cf0fe6f021..a961430d32 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 - 33.2.0 + 33.3.0 enable preview 1.0.0 diff --git a/src/Directory.Packages.props b/src/Directory.Packages.props index 38dc5cbb35..d400efcfa2 100644 --- a/src/Directory.Packages.props +++ b/src/Directory.Packages.props @@ -7,7 +7,7 @@ - + @@ -23,7 +23,7 @@ - + diff --git a/src/ModuleInitDocs/PagedDocuments.cs b/src/ModuleInitDocs/PagedDocuments.cs new file mode 100644 index 0000000000..8b5f8ed223 --- /dev/null +++ b/src/ModuleInitDocs/PagedDocuments.cs @@ -0,0 +1,17 @@ +public class PagedDocuments +{ + #region StaticPagedDocuments + + public static class ModuleInitializer + { + [ModuleInitializer] + public static void Init() + { + VerifierSettings.PageText(PageTextPlacement.PerPage); + VerifierSettings.PagesToInclude(10); + VerifierSettings.ExcludeDerivedTargets("png"); + } + } + + #endregion +} diff --git a/src/StaticSettingsTests/InlineSwitchOffTests.cs b/src/StaticSettingsTests/InlineSwitchOffTests.cs index 3245ad40b1..1a2e93f233 100644 --- a/src/StaticSettingsTests/InlineSwitchOffTests.cs +++ b/src/StaticSettingsTests/InlineSwitchOffTests.cs @@ -430,7 +430,7 @@ public async Task AnUnreadableRecordIsLeftForALaterRun() var record = Assert.Single(Records()); VerifierSettings.Reset(); - using (new FileStream(record, FileMode.Open, FileAccess.Read, FileShare.Delete)) + await using (new FileStream(record, FileMode.Open, FileAccess.Read, FileShare.Delete)) { await Verify("value", PassingSettings()); } diff --git a/src/StaticSettingsTests/PageSettingsTests.cs b/src/StaticSettingsTests/PageSettingsTests.cs new file mode 100644 index 0000000000..81da1d5d9d --- /dev/null +++ b/src/StaticSettingsTests/PageSettingsTests.cs @@ -0,0 +1,106 @@ +// The settings a converter of a paged document reads, set for every verification. Lives here since +// they are static, and this project runs serially with BaseTest resetting between tests. +public class PageSettingsTests : + BaseTest +{ + [Fact] + public void NothingSetIsEveryPageWithItsTextInTheInfo() + { + var context = new VerifySettings().Context; + + Assert.Equal(PageTextPlacement.InInfo, context.PageTextPlacement()); + Assert.True(context.IsPageIncluded(1)); + Assert.True(context.IsPageIncluded(500)); + Assert.False(context.IsDerivedTargetExcluded("png")); + } + + [Fact] + public void GlobalSettingsAreReadFromAnyContext() + { + VerifierSettings.PageText(PageTextPlacement.PerPage); + VerifierSettings.PagesToInclude(2); + VerifierSettings.ExcludeDerivedTargets("png"); + + var context = new VerifySettings().Context; + + Assert.Equal(PageTextPlacement.PerPage, context.PageTextPlacement()); + Assert.True(context.IsPageIncluded(2)); + Assert.False(context.IsPageIncluded(3)); + Assert.True(context.IsDerivedTargetExcluded("png")); + Assert.True(context.IsDerivedTargetExcluded("PNG")); + Assert.False(context.IsDerivedTargetExcluded("txt")); + // Only what was derived. A png that is the thing being verified is not excluded + Assert.False(context.IsTargetExcluded("png")); + } + + [Fact] + public void AGlobalFilterCanBeAnyPredicate() + { + VerifierSettings.PagesToInclude(_ => _ % 2 == 0); + + var context = new VerifySettings().Context; + + Assert.False(context.IsPageIncluded(1)); + Assert.True(context.IsPageIncluded(2)); + } + + /// + /// The verification's own setting is the one used, and its exclusions add to the global ones. + /// + [Fact] + public void TheVerificationsSettingWins() + { + VerifierSettings.PageText(PageTextPlacement.PerPage); + VerifierSettings.PagesToInclude(1); + VerifierSettings.ExcludeDerivedTargets("png"); + + var settings = new VerifySettings(); + settings.PageText(PageTextPlacement.None); + settings.PagesToInclude(3); + settings.ExcludeDerivedTargets("csv"); + var context = settings.Context; + + Assert.Equal(PageTextPlacement.None, context.PageTextPlacement()); + Assert.True(context.IsPageIncluded(3)); + Assert.True(context.IsDerivedTargetExcluded("png")); + Assert.True(context.IsDerivedTargetExcluded("csv")); + } + + /// + /// Excluding an extension outright excludes what was derived with it too, so a converter + /// asking about a derived target hears of either. + /// + [Fact] + public void ExcludingAnExtensionExcludesWhatIsDerivedWithIt() + { + VerifierSettings.ExcludeTargets("png"); + + Assert.True(new VerifySettings().Context.IsDerivedTargetExcluded("png")); + } + + [Fact] + public void ACopyOfSettingsKeepsThem() + { + var settings = new VerifySettings(); + settings.PageText(PageTextPlacement.PerPage); + settings.PagesToInclude(1); + settings.ExcludeDerivedTargets("png"); + + var context = new VerifySettings(settings).Context; + + Assert.Equal(PageTextPlacement.PerPage, context.PageTextPlacement()); + Assert.False(context.IsPageIncluded(2)); + Assert.True(context.IsDerivedTargetExcluded("png")); + } + + [Fact] + public void AfterVerifyHasBeenRunThrows() + { + InnerVerifier.verifyHasBeenRun = true; + + Assert.Throws(() => VerifierSettings.PageText(PageTextPlacement.PerPage)); + Assert.Throws(() => VerifierSettings.PagesToInclude(1)); + Assert.Throws(() => VerifierSettings.PagesToInclude(_ => true)); + Assert.Throws(() => VerifierSettings.ExcludeDerivedTargets("png")); + } +} diff --git a/src/StaticSettingsTests/RaisedDeleteTests.cs b/src/StaticSettingsTests/RaisedDeleteTests.cs index b83600bc71..bf992fcbd4 100644 --- a/src/StaticSettingsTests/RaisedDeleteTests.cs +++ b/src/StaticSettingsTests/RaisedDeleteTests.cs @@ -13,7 +13,7 @@ public class RaisedDeleteTests : { List added = []; List settled = []; - Func originalAddDelete = RaisedDeletes.AddDelete; + Func originalAddDelete = RaisedDeletes.AddDelete; Action originalSettleDelete = RaisedDeletes.SettleDelete; bool originalDisabled = DiffRunner.Disabled; TempDirectory temp = new(); @@ -21,9 +21,9 @@ public class RaisedDeleteTests : public RaisedDeleteTests() { // Both stood in for, so nothing reaches whatever tray or viewer is running on this machine - RaisedDeletes.AddDelete = _ => + RaisedDeletes.AddDelete = (file, _) => { - added.Add(_); + added.Add(file); return Task.CompletedTask; }; RaisedDeletes.SettleDelete = _ => settled.Add(_); diff --git a/src/StaticSettingsTests/ReceivedMapTests.cs b/src/StaticSettingsTests/ReceivedMapTests.cs index 7c4aefe8c3..0eba970b80 100644 --- a/src/StaticSettingsTests/ReceivedMapTests.cs +++ b/src/StaticSettingsTests/ReceivedMapTests.cs @@ -130,7 +130,8 @@ static List MapFiles() => foreach (var file in MapFiles()) { var lines = File.ReadAllLines(file); - if (lines.Length == 2) + // A third line names the document a file was derived from: SourceDerivedReportTests + if (lines.Length >= 2) { records.Add((lines[0], lines[1])); } diff --git a/src/StaticSettingsTests/SourceDerivedReportTests.cs b/src/StaticSettingsTests/SourceDerivedReportTests.cs new file mode 100644 index 0000000000..70cc3846ec --- /dev/null +++ b/src/StaticSettingsTests/SourceDerivedReportTests.cs @@ -0,0 +1,413 @@ +using DiffEngine; + +// Lives here, rather than in Verify.Tests, since what reaches a diff tool is observed through +// static seams, and this project runs serially with BaseTest resetting between tests. +// +// A converter that tells its document from what it derived from it has the derived files reported +// as such, so a tool drawing the document can show them beneath it and accept the lot as one. A +// file names its source only while that source is itself pending, and after the source has been +// reported. +public class SourceDerivedReportTests : + BaseTest, + IDisposable +{ + // Everything that would have gone to DiffEngine, in the order it would have gone + List reported = []; + Func originalLaunchDiff = VerifyEngine.LaunchDiff; + Func originalAddDelete = RaisedDeletes.AddDelete; + Action originalSettleDelete = RaisedDeletes.SettleDelete; + Func> originalAddInline = InlineEngine.AddInline; + Action originalSendRetire = InlineEngine.SendRetire; + bool originalDisabled = DiffRunner.Disabled; + TempDirectory temp = new(); + + public SourceDerivedReportTests() + { + // All stood in for, so nothing reaches whatever tray or viewer is running on this machine + VerifyEngine.LaunchDiff = (file, source) => + { + reported.Add($"move {Path.GetFileName(file.ReceivedPath)}{From(source)}"); + return Task.CompletedTask; + }; + RaisedDeletes.AddDelete = (file, source) => + { + reported.Add($"delete {Path.GetFileName(file)}{From(source)}"); + return Task.CompletedTask; + }; + RaisedDeletes.SettleDelete = _ => + { + }; + + // A snapshot the global switch queues is a patch against this file, and one accepted in + // the viewer writes a Snapshot call into the test that queued it + InlineEngine.AddInline = _ => Task.FromResult(InlineResult.Queued); + InlineEngine.SendRetire = (_, _, _) => + { + }; + + // DiffEngine switches itself off on a build server, under continuous testing and under an + // AI CLI, and nothing is launched while it is off + DiffRunner.Disabled = false; + BuildServerDetector.Detected = false; + + // The shape the document plugins have: the document, with a page of it drawn and read + VerifierSettings.RegisterStreamConverter( + "rdoc", + (_, stream, _) => + { + var content = Read(stream); + return new( + $"info of {content}", + new Target("rdoc", Stream(content)), + [ + new("rpage", Stream($"page of {content}"), "page_0001"), + new("txt", $"text of {content}", "page_0001") + ]); + }); + + // A document that is rendered to another, which is converted in its turn + VerifierSettings.RegisterStreamConverter( + "router", + (_, stream, _) => + { + var content = Read(stream); + return new( + null, + new Target("router", Stream(content)), + [new("rdoc", Stream($"inner of {content}"))]); + }); + } + + public void Dispose() + { + VerifyEngine.LaunchDiff = originalLaunchDiff; + RaisedDeletes.AddDelete = originalAddDelete; + RaisedDeletes.SettleDelete = originalSettleDelete; + InlineEngine.AddInline = originalAddInline; + InlineEngine.SendRetire = originalSendRetire; + DiffRunner.Disabled = originalDisabled; + temp.Dispose(); + } + + /// + /// The info is the first target and the document the second, and the document is still the + /// first thing a diff tool hears of. + /// + [Fact] + public async Task ASourceIsReportedAheadOfWhatWasDerivedFromIt() + { + await Assert.ThrowsAsync(() => Verify(Stream("doc"), "rdoc", Settings())); + + Assert.Equal( + [ + "move Shared.received.rdoc", + "move Shared.received.txt from Shared.received.rdoc", + "move Shared#page_0001.received.rpage from Shared.received.rdoc", + "move Shared#page_0001.received.txt from Shared.received.rdoc" + ], + reported); + } + + /// + /// What is reported in another order is still handed to the callbacks, and listed in the + /// exception, in the order the targets came in. + /// + [Fact] + public async Task CallbacksAndTheExceptionKeepTheOrderOfTheTargets() + { + var called = new List(); + var settings = Settings(); + settings.OnFirstVerify( + (file, _, _) => + { + called.Add(Path.GetFileName(file.ReceivedPath)); + // Nothing has been reported while the callbacks run + Assert.Empty(reported); + return Task.CompletedTask; + }); + + var exception = await Assert.ThrowsAsync(() => Verify(Stream("doc"), "rdoc", settings)); + + string[] expected = + [ + "Shared.received.txt", + "Shared.received.rdoc", + "Shared#page_0001.received.rpage", + "Shared#page_0001.received.txt" + ]; + Assert.Equal(expected, called); + var positions = expected + .Select(_ => exception.Message.IndexOf($"Received: {_}", StringComparison.Ordinal)) + .ToList(); + Assert.DoesNotContain(-1, positions); + Assert.Equal(positions.OrderBy(_ => _), positions); + } + + /// + /// A document that has not changed has no received file to name, and no row in a tool for a + /// file to be shown beneath. + /// + [Fact] + public async Task ASourceThatPassedIsNotNamed() + { + await File.WriteAllTextAsync(Path.Combine(temp.Path, "Shared.verified.rdoc"), "doc"); + + await Assert.ThrowsAsync(() => Verify(Stream("doc"), "rdoc", Settings())); + + Assert.Equal( + [ + "move Shared.received.txt", + "move Shared#page_0001.received.rpage", + "move Shared#page_0001.received.txt" + ], + reported); + } + + [Fact] + public async Task ASourceThatWasAutoVerifiedIsNotNamed() + { + var settings = Settings(); + settings.AutoVerify(_ => _.EndsWith(".rdoc", StringComparison.Ordinal)); + + await Assert.ThrowsAsync(() => Verify(Stream("doc"), "rdoc", settings)); + + Assert.Equal( + [ + "move Shared.received.txt", + "move Shared#page_0001.received.rpage", + "move Shared#page_0001.received.txt" + ], + reported); + } + + /// + /// An info passed to the verification is the test's, so the file holding it is not something + /// the document alone accounts for. + /// + [Fact] + public async Task AnInfoTheTestAddedToIsNotDerived() + { + await Assert.ThrowsAsync(() => Verify(Stream("doc"), "rdoc", Settings(), info: "from the test")); + + Assert.Equal( + [ + "move Shared.received.txt", + "move Shared.received.rdoc", + "move Shared#page_0001.received.rpage from Shared.received.rdoc", + "move Shared#page_0001.received.txt from Shared.received.rdoc" + ], + reported); + } + + /// + /// One level is all a diff tool is told. A page of a document that was rendered from another + /// is a file of the outer one, as the inner document is. + /// + [Fact] + public async Task ANestedSourceNamesTheOutermostOne() + { + await Assert.ThrowsAsync(() => Verify(Stream("outer"), "router", Settings())); + + Assert.Equal( + [ + "move Shared.received.router", + "move Shared.received.txt from Shared.received.router", + "move Shared.received.rdoc from Shared.received.router", + "move Shared#page_0001.received.rpage from Shared.received.router", + "move Shared#page_0001.received.txt from Shared.received.router" + ], + reported); + } + + /// + /// With the outer document unchanged, the inner one is the outermost that is pending. + /// + [Fact] + public async Task ANestedSourceIsNamedWhenTheOneAboveItPassed() + { + await File.WriteAllTextAsync(Path.Combine(temp.Path, "Shared.verified.router"), "outer"); + + await Assert.ThrowsAsync(() => Verify(Stream("outer"), "router", Settings())); + + Assert.Equal( + [ + "move Shared.received.txt", + "move Shared.received.rdoc", + "move Shared#page_0001.received.rpage from Shared.received.rdoc", + "move Shared#page_0001.received.txt from Shared.received.rdoc" + ], + reported); + } + + /// + /// A page the document no longer has. There is no target to say what it came from, so it goes + /// by there being one document pending, and follows the launch for it. + /// + [Fact] + public async Task AStaleFileIsDeletedWithTheOneDocument() + { + await File.WriteAllTextAsync(Path.Combine(temp.Path, "Shared#page_0002.verified.rpage"), "a page the document had"); + + await Assert.ThrowsAsync(() => Verify(Stream("doc"), "rdoc", Settings())); + + Assert.Equal("move Shared.received.rdoc", reported[0]); + Assert.Equal("delete Shared#page_0002.verified.rpage from Shared.received.rdoc", reported[^1]); + Assert.Equal(5, reported.Count); + } + + /// + /// With the document unchanged the delete stands alone, and is raised ahead of the moves as + /// a delete always was. + /// + [Fact] + public async Task AStaleFileOfADocumentThatPassedStandsAlone() + { + await File.WriteAllTextAsync(Path.Combine(temp.Path, "Shared.verified.rdoc"), "doc"); + await File.WriteAllTextAsync(Path.Combine(temp.Path, "Shared#page_0002.verified.rpage"), "a page the document had"); + + await Assert.ThrowsAsync(() => Verify(Stream("doc"), "rdoc", Settings())); + + Assert.Equal("delete Shared#page_0002.verified.rpage", reported[0]); + Assert.Equal(4, reported.Count); + } + + /// + /// With more than one document pending, a stale file belongs to the one its name carries on + /// from, and to none where it carries on from neither. + /// + [Fact] + public async Task AStaleFileIsDeletedWithTheDocumentItIsNamedAfter() + { + await File.WriteAllTextAsync(Path.Combine(temp.Path, "Shared#Attachment2.page_0002.verified.rpage"), "a page the second had"); + await File.WriteAllTextAsync(Path.Combine(temp.Path, "Shared#Another.verified.txt"), "of neither"); + List attachments = + [ + new("rdoc", Stream("first"), "Attachment1"), + new("rdoc", Stream("second"), "Attachment2") + ]; + + await Assert.ThrowsAsync(() => Verify(attachments, Settings())); + + Assert.Equal("delete Shared#Another.verified.txt", reported[0]); + Assert.Equal("delete Shared#Attachment2.page_0002.verified.rpage from Shared#Attachment2.received.rdoc", reported[^1]); + Assert.Equal( + [ + "move Shared#Attachment1.received.rdoc", + "move Shared#Attachment2.received.rdoc" + ], + reported.Where(_ => _.StartsWith("move", StringComparison.Ordinal) && !_.Contains(" from "))); + Assert.Contains("move Shared#Attachment1.received.txt from Shared#Attachment1.received.rdoc", reported); + Assert.Contains("move Shared#Attachment2.page_0001.received.rpage from Shared#Attachment2.received.rdoc", reported); + } + + /// + /// With diff off for the verification no tool is told of the document, so there is nothing + /// for a delete to be shown beneath. The delete itself is raised as it always was. + /// + [Fact] + public async Task WithDiffOffNothingIsLaunchedAndADeleteStandsAlone() + { + await File.WriteAllTextAsync(Path.Combine(temp.Path, "Shared#page_0002.verified.rpage"), "a page the document had"); + var settings = Settings(); + settings.DisableDiff(); + + await Assert.ThrowsAsync(() => Verify(Stream("doc"), "rdoc", settings)); + + Assert.Equal(["delete Shared#page_0002.verified.rpage"], reported); + } + + /// + /// The map is for tooling that finds the files on disk, where the document is as much there + /// as its pages are, whatever a diff tool was told. + /// + [Fact] + public async Task TheReceivedMapNamesTheSource() + { + var settings = Settings(); + settings.DisableDiff(); + + await Assert.ThrowsAsync(() => Verify(Stream("doc"), "rdoc", settings)); + + var document = Path.Combine(temp.Path, "Shared.received.rdoc"); + Assert.Equal( + [ + document, + Path.Combine(temp.Path, "Shared.verified.rdoc") + ], + Map(document)); + Assert.Equal( + [ + Path.Combine(temp.Path, "Shared#page_0001.received.rpage"), + Path.Combine(temp.Path, "Shared#page_0001.verified.rpage"), + document + ], + Map(Path.Combine(temp.Path, "Shared#page_0001.received.rpage"))); + Assert.Equal(document, Map(Path.Combine(temp.Path, "Shared.received.txt"))[2]); + } + + /// + /// The global inline switch takes the first target, which for a document is its info. That + /// is a file of the document, so it stays one, whether or not the document itself is a target. + /// The same info from a converter that tells no source from what it derived is inlined, as it + /// always was, which is what shows the other two were declined for being a document's. + /// + [Fact] + public async Task TheInfoOfADocumentIsNotInlinedByTheGlobalSwitch() + { + VerifierSettings.RegisterStreamConverter( + "rsheets", + (_, _, _) => new("info of the sheets", null, [new("txt", "a,b", "Sheet1")])); + VerifierSettings.RegisterStreamConverter( + "rplain", + (_, _, _) => new("info of the plain", [new("txt", "a,b", "Sheet1")])); + VerifierSettings.Inline(); + + var document = await Assert.ThrowsAsync(() => Verify(Stream("doc"), "rdoc", Settings())); + Assert.DoesNotContain("InlineNew:", document.Message); + + var sheets = await Assert.ThrowsAsync(() => Verify(Stream("doc"), "rsheets", Settings())); + Assert.DoesNotContain("InlineNew:", sheets.Message); + + var plain = await Assert.ThrowsAsync(() => Verify(Stream("doc"), "rplain", Settings())); + Assert.Contains("InlineNew:", plain.Message); + } + + static MemoryStream Stream(string content) => + new(Encoding.UTF8.GetBytes(content)); + + static string Read(Stream stream) + { + using var reader = new StreamReader(stream); + return reader.ReadToEnd(); + } + + static string From(string? source) + { + if (source is null) + { + return ""; + } + + return $" from {Path.GetFileName(source)}"; + } + + VerifySettings Settings() + { + var settings = new VerifySettings(); + settings.UseDirectory(temp); + settings.UseFileName("Shared"); + settings.DisableRequireUniquePrefix(); + return settings; + } + + // Maps are named from a hash of the received path, so they are located by content + static string[] Map(string receivedPath) + { + var directory = Path.Combine( + AttributeReader.GetIntermediateDirectory(typeof(SourceDerivedReportTests).Assembly), + "VerifyReceived"); + return Directory.EnumerateFiles(directory) + .Select(File.ReadAllLines) + .Single(_ => _.Length > 0 && _[0] == receivedPath); + } +} diff --git a/src/Verify.ExceptionParsing.Tests/ReceivedMapsTests.cs b/src/Verify.ExceptionParsing.Tests/ReceivedMapsTests.cs index c1d41303d3..60f19dee21 100644 --- a/src/Verify.ExceptionParsing.Tests/ReceivedMapsTests.cs +++ b/src/Verify.ExceptionParsing.Tests/ReceivedMapsTests.cs @@ -147,10 +147,111 @@ public void LookupNormalizesPaths() Assert.Equal(verified, found); } - static void WriteMap(string parent, string received, string verified) + [ModuleInitializer] + public static void Init() => + VerifierSettings.RegisterStreamConverter( + "mapdoc", + (_, _, _) => + new( + null, + new Target("mapdoc", new MemoryStream("the document"u8.ToArray())), + [new("mappage", new MemoryStream("the page"u8.ToArray()), "page_0001")])); + + // Pins the third line of a map between the writer, in Verify, and the reader here. + [Fact] + public async Task ReadsSourceWrittenByVerify() + { + var detected = DiffEngine.BuildServerDetector.Detected; + DiffEngine.BuildServerDetector.Detected = false; + try + { + using var temp = new TempDirectory(); + var settings = new VerifySettings(); + settings.UseDirectory(temp); + settings.DisableDiff(); + + await Assert.ThrowsAsync(() => VerifyXunit.Verifier.Verify(new MemoryStream("input"u8.ToArray()), "mapdoc", settings)); + + var files = Directory.EnumerateFiles(temp).ToList(); + var document = files.Single(_ => _.EndsWith(".received.mapdoc", StringComparison.Ordinal)); + var page = files.Single(_ => _.EndsWith(".received.mappage", StringComparison.Ordinal)); + + var maps = ReceivedMaps.Read(AttributeReader.GetIntermediateDirectory()); + + Assert.True(maps.TryGetSource(page, out var source)); + Assert.Equal(document, source); + Assert.False(maps.TryGetSource(document, out _)); + Assert.True(maps.TryGetVerified(page, out var verified)); + Assert.Equal( + Path.Combine(temp.Path, "ReceivedMapsTests.ReadsSourceWrittenByVerify#page_0001.verified.mappage"), + verified); + } + finally + { + DiffEngine.BuildServerDetector.Detected = detected; + } + } + + [Fact] + public void ADerivedFileNamesItsSource() + { + using var temp = new TempDirectory(); + var document = Path.Combine(temp.Path, "Foo.received.pdf"); + var page = Path.Combine(temp.Path, "Foo#page_0001.received.png"); + var verifiedPage = Path.Combine(temp.Path, "Foo#page_0001.verified.png"); + File.WriteAllText(document, "the document"); + File.WriteAllText(page, "the page"); + var obj = Path.Combine(temp.Path, "obj"); + WriteMap(obj, "document.txt", document, Path.Combine(temp.Path, "Foo.verified.pdf")); + WriteMap(obj, "page.txt", page, verifiedPage, document); + + var maps = ReceivedMaps.Read(temp); + + Assert.Equal(2, maps.Pairs.Count); + Assert.True(maps.TryGetSource(page, out var source)); + Assert.Equal(document, source); + // The document is derived from nothing, and the pair of the page is what it always was + Assert.False(maps.TryGetSource(document, out _)); + Assert.True(maps.TryGetVerified(page, out var found)); + Assert.Equal(verifiedPage, found); + } + + /// + /// A document accepted on its own leaves its pages pending with a record that still names it. + /// They stand alone then, since there is nothing left to accept them with. + /// + [Fact] + public void ASourceThatHasGoneIsNotNamed() + { + using var temp = new TempDirectory(); + var page = Path.Combine(temp.Path, "Foo#page_0001.received.png"); + File.WriteAllText(page, "the page"); + WriteMap( + Path.Combine(temp.Path, "obj"), + "page.txt", + page, + Path.Combine(temp.Path, "Foo#page_0001.verified.png"), + Path.Combine(temp.Path, "Foo.received.pdf")); + + var maps = ReceivedMaps.Read(temp); + + Assert.Single(maps.Pairs); + Assert.False(maps.TryGetSource(page, out _)); + } + + static void WriteMap(string parent, string received, string verified) => + WriteMap(parent, "map.txt", received, verified); + + static void WriteMap(string parent, string name, string received, string verified, string? source = null) { var directory = Path.Combine(parent, "VerifyReceived"); Directory.CreateDirectory(directory); - File.WriteAllLines(Path.Combine(directory, "map.txt"), [received, verified]); + List lines = [received, verified]; + if (source is not null) + { + lines.Add(source); + } + + File.WriteAllLines(Path.Combine(directory, name), lines); } } diff --git a/src/Verify.ExceptionParsing/ReceivedMaps.cs b/src/Verify.ExceptionParsing/ReceivedMaps.cs index 00e19fa7f8..0a437561ff 100644 --- a/src/Verify.ExceptionParsing/ReceivedMaps.cs +++ b/src/Verify.ExceptionParsing/ReceivedMaps.cs @@ -13,6 +13,10 @@ namespace VerifyTests.ExceptionParsing; /// The records are written to the intermediate (obj) directory of the test project, in a /// `VerifyReceived` directory, one file per received file, each holding the received path on the first /// line and the verified path on the second. +/// +/// A file that a converter derived from a document, such as the image of a page, has a third line +/// while that document is itself pending: the received path of the document. See +/// . /// public sealed class ReceivedMaps { @@ -25,11 +29,13 @@ public sealed class ReceivedMaps : StringComparer.Ordinal; readonly Dictionary verifiedByReceived; + readonly Dictionary sourceByReceived; - ReceivedMaps(IReadOnlyList pairs, Dictionary verifiedByReceived) + ReceivedMaps(IReadOnlyList pairs, Dictionary verifiedByReceived, Dictionary sourceByReceived) { Pairs = pairs; this.verifiedByReceived = verifiedByReceived; + this.sourceByReceived = sourceByReceived; } /// @@ -46,12 +52,13 @@ public sealed class ReceivedMaps public static ReceivedMaps Read(string directory) { var lookup = new Dictionary(pathComparer); + var sources = new Dictionary(pathComparer); foreach (var mapDirectory in FindMapDirectories(directory)) { foreach (var file in EnumerateFiles(mapDirectory)) { - if (!TryReadPair(file, out var pair)) + if (!TryReadPair(file, out var pair, out var source)) { continue; } @@ -69,6 +76,21 @@ public static ReceivedMaps Read(string directory) // The same received path can be recorded by more than one build, for example Debug // and Release, with the same result. So the last wins rather than throwing. lookup[received] = pair.Verified; + + // The last wins here too, including where the last names no source + sources.Remove(received); + if (source is null) + { + continue; + } + + // A document that was accepted on its own has left what was derived from it + // standing alone, the same way a record outlives its received file. + source = Normalize(source); + if (File.Exists(source)) + { + sources[received] = source; + } } } @@ -80,7 +102,7 @@ public static ReceivedMaps Read(string directory) pairs.Add(new(entry.Key, entry.Value)); } - return new(pairs, lookup); + return new(pairs, lookup, sources); } /// @@ -92,6 +114,19 @@ public static ReceivedMaps Read(string directory) public bool TryGetVerified(string receivedPath, [NotNullWhen(true)] out string? verified) => verifiedByReceived.TryGetValue(Normalize(receivedPath), out verified); + /// + /// Finds the received file of the document that was derived + /// from by a converter: the pdf that the image of a page was rendered from. A tool can then + /// present the document and what was derived from it as one change, and accept them together. + /// + /// + /// False for a file that stands alone: one that was not derived from a document, the document + /// itself, and a file whose document is no longer pending. One level only, so the source of a + /// file never has a source of its own. + /// + public bool TryGetSource(string receivedPath, [NotNullWhen(true)] out string? source) => + sourceByReceived.TryGetValue(Normalize(receivedPath), out source); + // A junction or symlink can make the tree cyclic, and netstandard2.0 has no way to resolve a link // target to detect that. So the walk is bounded instead. An intermediate directory sits only a // handful of levels below the project or repository root it is scanned from. @@ -161,8 +196,9 @@ static IEnumerable EnumerateFiles(string directory) } } - static bool TryReadPair(string file, out FilePair pair) + static bool TryReadPair(string file, out FilePair pair, out string? source) { + source = null; string[] lines; try { @@ -175,7 +211,8 @@ static bool TryReadPair(string file, out FilePair pair) return false; } - // Only the first two lines are read, so that a future version adding more does not break this. + // Lines past the ones known here are ignored, so that a future version adding more does + // not break this. if (lines.Length < 2 || lines[0].Length == 0 || lines[1].Length == 0) @@ -185,6 +222,12 @@ static bool TryReadPair(string file, out FilePair pair) } pair = new(lines[0], lines[1]); + if (lines.Length > 2 && + lines[2].Length != 0) + { + source = lines[2]; + } + return true; } diff --git a/src/Verify.MSTest.SourceGenerator.Tests/GlobalNamespaceTests.cs b/src/Verify.MSTest.SourceGenerator.Tests/GlobalNamespaceTests.cs index e3b3b786be..a184a12a1c 100644 --- a/src/Verify.MSTest.SourceGenerator.Tests/GlobalNamespaceTests.cs +++ b/src/Verify.MSTest.SourceGenerator.Tests/GlobalNamespaceTests.cs @@ -1,4 +1,5 @@ [TestClass] +// ReSharper disable once PartialTypeWithSinglePart public partial class GlobalNamespaceTests : TestBase { [TestMethod] diff --git a/src/Verify.MSTest.SourceGenerator.Tests/InheritanceTests.cs b/src/Verify.MSTest.SourceGenerator.Tests/InheritanceTests.cs index 157ee75188..b5835bb16e 100644 --- a/src/Verify.MSTest.SourceGenerator.Tests/InheritanceTests.cs +++ b/src/Verify.MSTest.SourceGenerator.Tests/InheritanceTests.cs @@ -1,3 +1,4 @@ +// ReSharper disable PartialTypeWithSinglePart [TestClass] public partial class InheritanceTests : TestBase { diff --git a/src/Verify.MSTest.SourceGenerator.Tests/NamespaceTests.cs b/src/Verify.MSTest.SourceGenerator.Tests/NamespaceTests.cs index f8a3c0e550..f57af45728 100644 --- a/src/Verify.MSTest.SourceGenerator.Tests/NamespaceTests.cs +++ b/src/Verify.MSTest.SourceGenerator.Tests/NamespaceTests.cs @@ -1,4 +1,5 @@ [TestClass] +// ReSharper disable once PartialTypeWithSinglePart public partial class NamespaceTests : TestBase { [TestMethod] diff --git a/src/Verify.MSTest.SourceGenerator.Tests/NoMatchTests.cs b/src/Verify.MSTest.SourceGenerator.Tests/NoMatchTests.cs index 37db4f743b..7afacd6a02 100644 --- a/src/Verify.MSTest.SourceGenerator.Tests/NoMatchTests.cs +++ b/src/Verify.MSTest.SourceGenerator.Tests/NoMatchTests.cs @@ -1,4 +1,5 @@ [TestClass] +// ReSharper disable once PartialTypeWithSinglePart public partial class NoMatchTests : TestBase { [TestMethod] diff --git a/src/Verify.MSTest.SourceGenerator.Tests/RecordTests.cs b/src/Verify.MSTest.SourceGenerator.Tests/RecordTests.cs index b2f93280ea..1e6b20bd8d 100644 --- a/src/Verify.MSTest.SourceGenerator.Tests/RecordTests.cs +++ b/src/Verify.MSTest.SourceGenerator.Tests/RecordTests.cs @@ -1,5 +1,6 @@ // A record is a class, so it is a legal MSTest test class [TestClass] +// ReSharper disable once PartialTypeWithSinglePart public partial class RecordTests : TestBase { [TestMethod] diff --git a/src/Verify.MSTest.SourceGenerator.Tests/StaticClassTests.cs b/src/Verify.MSTest.SourceGenerator.Tests/StaticClassTests.cs index 793aa9e775..dbc374bd56 100644 --- a/src/Verify.MSTest.SourceGenerator.Tests/StaticClassTests.cs +++ b/src/Verify.MSTest.SourceGenerator.Tests/StaticClassTests.cs @@ -1,4 +1,5 @@ [TestClass] +// ReSharper disable once PartialTypeWithSinglePart public partial class StaticClassTests : TestBase { /// diff --git a/src/Verify.MSTest/DerivePaths/Verifier_Obsoletes.cs b/src/Verify.MSTest/DerivePaths/Verifier_Obsoletes.cs index cf1c78050e..246146987b 100644 --- a/src/Verify.MSTest/DerivePaths/Verifier_Obsoletes.cs +++ b/src/Verify.MSTest/DerivePaths/Verifier_Obsoletes.cs @@ -7,5 +7,6 @@ public partial class Verifier /// [Obsolete("Use the overload that accepts mirrorSourceStructure.")] public static void UseProjectRelativeDirectory(string directory) => + // ReSharper disable once RedundantArgumentDefaultValue UseProjectRelativeDirectory(directory, false); } diff --git a/src/Verify.MSTest/DerivePaths/VerifyBase_Obsoletes.cs b/src/Verify.MSTest/DerivePaths/VerifyBase_Obsoletes.cs index b182886e7b..135f73c716 100644 --- a/src/Verify.MSTest/DerivePaths/VerifyBase_Obsoletes.cs +++ b/src/Verify.MSTest/DerivePaths/VerifyBase_Obsoletes.cs @@ -7,5 +7,6 @@ public partial class VerifyBase /// [Obsolete("Use the overload that accepts mirrorSourceStructure.")] public static void UseProjectRelativeDirectory(string directory) => + // ReSharper disable once RedundantArgumentDefaultValue Verifier.UseProjectRelativeDirectory(directory, false); } diff --git a/src/Verify.Tests/Converters/PagedConversionTests.Sample#page_0001.verified.png b/src/Verify.Tests/Converters/PagedConversionTests.Sample#page_0001.verified.png new file mode 100644 index 0000000000..f37764b1f7 Binary files /dev/null and b/src/Verify.Tests/Converters/PagedConversionTests.Sample#page_0001.verified.png differ diff --git a/src/Verify.Tests/Converters/PagedConversionTests.Sample#page_0002.verified.png b/src/Verify.Tests/Converters/PagedConversionTests.Sample#page_0002.verified.png new file mode 100644 index 0000000000..613754cfaf Binary files /dev/null and b/src/Verify.Tests/Converters/PagedConversionTests.Sample#page_0002.verified.png differ diff --git a/src/Verify.Tests/Converters/PagedConversionTests.Sample.verified.slides b/src/Verify.Tests/Converters/PagedConversionTests.Sample.verified.slides new file mode 100644 index 0000000000..6fcee8119b --- /dev/null +++ b/src/Verify.Tests/Converters/PagedConversionTests.Sample.verified.slides @@ -0,0 +1 @@ +The title|The first page|The second page \ No newline at end of file diff --git a/src/Verify.Tests/Converters/PagedConversionTests.Sample.verified.txt b/src/Verify.Tests/Converters/PagedConversionTests.Sample.verified.txt new file mode 100644 index 0000000000..be52c4588a --- /dev/null +++ b/src/Verify.Tests/Converters/PagedConversionTests.Sample.verified.txt @@ -0,0 +1,16 @@ +{ + Document: { + Title: The title + }, + PageCount: 2, + Pages: [ + { + Number: 1, + Text: The first page + }, + { + Number: 2, + Text: The second page + } + ] +} \ No newline at end of file diff --git a/src/Verify.Tests/Converters/PagedConversionTests.cs b/src/Verify.Tests/Converters/PagedConversionTests.cs new file mode 100644 index 0000000000..38162c9e95 --- /dev/null +++ b/src/Verify.Tests/Converters/PagedConversionTests.cs @@ -0,0 +1,510 @@ +// ReSharper disable UnusedParameter.Local +public class PagedConversionTests +{ + [ModuleInitializer] + public static void Init() + { + VerifierSettings.RegisterStreamConverter("slides", ConvertSlides); + VerifierSettings.RegisterStreamConverter("report", ConvertReport); + VerifierSettings.RegisterStreamConverter("sheets", ConvertSheets); + } + + #region PagedConversion + + // A document that is a title, then the text of each page, with a | between them + static ConversionResult ConvertSlides(string? name, Stream stream, IReadOnlyDictionary context) + { + var bytes = ReadBytes(stream); + var lines = Encoding.UTF8.GetString(bytes).Split('|'); + + var conversion = new PagedConversion(context) + { + Info = new + { + Title = lines[0] + } + }; + + // The document itself, unless ExcludeTargets says it is not wanted + if (!context.IsTargetExcluded("slides")) + { + conversion.Source(new("slides", new MemoryStream(bytes))); + } + + // The pages PagesToInclude asks for, which is all of them by default + foreach (var number in conversion.Pages(pageCount: lines.Length - 1)) + { + // Rendering and reading are the expensive parts, so ask before doing either + Stream? image = null; + if (conversion.IncludeImages) + { + image = Render(number); + } + + string? text = null; + if (conversion.IncludeText) + { + text = ReadText(lines[number]); + } + + conversion.AddPage(number, image, text); + } + + return conversion.Build(); + } + + #endregion + + // A renderer that draws every page at once, and reads the document as one text + static ConversionResult ConvertReport(string? name, Stream stream, IReadOnlyDictionary context) + { + var bytes = ReadBytes(stream); + var conversion = new PagedConversion(context) + { + TextExtension = "md" + }; + conversion.Source(new("report", new MemoryStream(bytes))); + reportImages = + [ + Render(1), + Render(2), + Render(3) + ]; + conversion.AddImages(reportImages); + conversion.Text($"# {Encoding.UTF8.GetString(bytes)}"); + return conversion.Build(); + } + + static ConversionResult ConvertSheets(string? name, Stream stream, IReadOnlyDictionary context) + { + var conversion = new PagedConversion(context); + conversion.Source(new("sheets", new MemoryStream(ReadBytes(stream)))); + conversion.AddDerived(new("csv", "a,b", "Sheet1")); + return conversion.Build(); + } + + static List reportImages = []; + static int rendered; + static int read; + + static byte[] ReadBytes(Stream stream) + { + var memory = new MemoryStream(); + stream.CopyTo(memory); + return memory.ToArray(); + } + + // A whole png of one pixel, so that what is committed as an image is one + static MemoryStream Render(int number) + { + Interlocked.Increment(ref rendered); + if (number % 2 == 1) + { + return new(Convert.FromBase64String("iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==")); + } + + return new(Convert.FromBase64String("iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChwGA60e6kgAAAABJRU5ErkJggg==")); + } + + static string ReadText(string line) + { + Interlocked.Increment(ref read); + return line; + } + + static MemoryStream Slides() => + new("The title|The first page|The second page"u8.ToArray()); + + #region PagedConversionVerify + + [Fact] + public Task Sample() => + Verify(Slides(), "slides"); + + #endregion + + [Fact] + public async Task TextInTheInfoByDefault() + { + using var temp = new TempDirectory(); + + var result = await Verify(Slides(), "slides") + .UseDirectory(temp) + .AutoVerify() + .DisableDiff(); + + Assert.Equal( + [ + "Test#page_0001.verified.png", + "Test#page_0002.verified.png", + "Test.verified.slides", + "Test.verified.txt" + ], + Names(result, nameof(TextInTheInfoByDefault))); + Assert.Contains("Text: The first page", Info(result)); + } + + [Fact] + public async Task TextPerPage() + { + using var temp = new TempDirectory(); + + var result = await Verify(Slides(), "slides") + .PageText(PageTextPlacement.PerPage) + .UseDirectory(temp) + .AutoVerify() + .DisableDiff(); + + Assert.Equal( + [ + "Test#page_0001.verified.png", + "Test#page_0001.verified.txt", + "Test#page_0002.verified.png", + "Test#page_0002.verified.txt", + "Test.verified.slides", + "Test.verified.txt" + ], + Names(result, nameof(TextPerPage))); + var info = Info(result); + Assert.DoesNotContain("Text", info); + Assert.Contains("PageCount: 2", info); + Assert.Equal( + "The second page", + await File.ReadAllTextAsync(result.Files.Single(_ => _.EndsWith("#page_0002.verified.txt")), Encoding.UTF8)); + } + + /// + /// With no text wanted the converter is told so, and does not read any. + /// + [Fact] + public async Task NoText() + { + using var temp = new TempDirectory(); + + read = 0; + var result = await Verify(Slides(), "slides") + .PageText(PageTextPlacement.None) + .UseDirectory(temp) + .AutoVerify() + .DisableDiff(); + + Assert.Equal(0, read); + Assert.Equal( + [ + "Test#page_0001.verified.png", + "Test#page_0002.verified.png", + "Test.verified.slides", + "Test.verified.txt" + ], + Names(result, nameof(NoText))); + Assert.DoesNotContain("Text", Info(result)); + } + + /// + /// The info still says how many pages the document has, and a page keeps its own number. + /// + [Fact] + public async Task TheFirstPages() + { + using var temp = new TempDirectory(); + + rendered = 0; + + var result = await Verify(Slides(), "slides") + .PagesToInclude(1) + .UseDirectory(temp) + .AutoVerify() + .DisableDiff(); + + Assert.Equal(1, rendered); + Assert.Equal( + [ + "Test#page_0001.verified.png", + "Test.verified.slides", + "Test.verified.txt" + ], + Names(result, nameof(TheFirstPages))); + var info = Info(result); + Assert.Contains("PageCount: 2", info); + Assert.DoesNotContain("The second page", info); + } + + [Fact] + public async Task ThePagesAskedFor() + { + using var temp = new TempDirectory(); + + var result = await Verify(Slides(), "slides") + .PagesToInclude(pageNumber => pageNumber == 2) + .UseDirectory(temp) + .AutoVerify() + .DisableDiff(); + + Assert.Equal( + [ + "Test#page_0002.verified.png", + "Test.verified.slides", + "Test.verified.txt" + ], + Names(result, nameof(ThePagesAskedFor))); + var info = Info(result); + Assert.Contains("Number: 2", info); + Assert.DoesNotContain("The first page", info); + } + + /// + /// With the images excluded the converter is told so, and renders nothing. + /// + [Fact] + public async Task NoImages() + { + using var temp = new TempDirectory(); + + rendered = 0; + + var result = await Verify(Slides(), "slides") + .ExcludeDerivedTargets("png") + .UseDirectory(temp) + .AutoVerify() + .DisableDiff(); + + Assert.Equal(0, rendered); + Assert.Equal( + [ + "Test.verified.slides", + "Test.verified.txt" + ], + Names(result, nameof(NoImages))); + } + + [Fact] + public async Task NoSource() + { + using var temp = new TempDirectory(); + + var result = await Verify(Slides(), "slides") + .ExcludeTargets("slides") + .UseDirectory(temp) + .AutoVerify() + .DisableDiff(); + + Assert.Equal( + [ + "Test#page_0001.verified.png", + "Test#page_0002.verified.png", + "Test.verified.txt" + ], + Names(result, nameof(NoSource))); + } + + /// + /// A converter that cannot say which page a part of the text is on gives the text whole. It + /// goes where the text of a page would have: the info, or a file of its own. + /// + [Fact] + public async Task TextOfTheWholeDocument() + { + using var temp = new TempDirectory(); + + var inInfo = await Verify(new MemoryStream("The report"u8.ToArray()), "report") + .UseDirectory(temp) + .AutoVerify() + .DisableDiff(); + Assert.Equal( + [ + "Test#page_0001.verified.png", + "Test#page_0002.verified.png", + "Test#page_0003.verified.png", + "Test.verified.report", + "Test.verified.txt" + ], + Names(inInfo, nameof(TextOfTheWholeDocument))); + var info = Info(inInfo); + Assert.Contains("Text: # The report", info); + Assert.Contains("PageCount: 3", info); + + var perPage = await Verify(new MemoryStream("The report"u8.ToArray()), "report") + .PageText(PageTextPlacement.PerPage) + .UseDirectory(temp) + .DisableRequireUniquePrefix() + .AutoVerify() + .DisableDiff(); + Assert.Contains( + "Test#text.verified.md", + Names(perPage, nameof(TextOfTheWholeDocument))); + Assert.DoesNotContain("Text", Info(perPage)); + } + + /// + /// A renderer that draws every page at once has drawn the ones that are not wanted too. They + /// are dropped, and their streams closed, since nothing else will. + /// + [Fact] + public async Task PagesDrawnAllAtOnceAreFilteredAfterwards() + { + using var temp = new TempDirectory(); + + var result = await Verify(new MemoryStream("The report"u8.ToArray()), "report") + .PagesToInclude(_ => _ != 2) + .UseDirectory(temp) + .AutoVerify() + .DisableDiff(); + + Assert.Equal( + [ + "Test#page_0001.verified.png", + "Test#page_0003.verified.png", + "Test.verified.report", + "Test.verified.txt" + ], + Names(result, nameof(PagesDrawnAllAtOnceAreFilteredAfterwards))); + Assert.Contains("PageCount: 3", Info(result)); + Assert.False(reportImages[1].CanRead); + } + + /// + /// Named by the converter, even when it is the only one: a second sheet added to a workbook + /// then adds a file rather than renaming the first. + /// + [Fact] + public async Task ADerivedTargetOfAnotherKind() + { + using var temp = new TempDirectory(); + + var result = await Verify(new MemoryStream("The workbook"u8.ToArray()), "sheets") + .UseDirectory(temp) + .AutoVerify() + .DisableDiff(); + + // Nothing to say of the document, so no info file + Assert.Equal( + [ + "Test#Sheet1.verified.csv", + "Test.verified.sheets" + ], + Names(result, nameof(ADerivedTargetOfAnotherKind))); + } + + [Fact] + public void ADerivedTargetRequiresAName() + { + var conversion = new PagedConversion(new VerifySettings().Context); + + Assert.Throws(() => conversion.AddDerived(new("csv", "a,b"))); + } + + [Fact] + public void PageNumbersAreOneBased() + { + var conversion = new PagedConversion(new VerifySettings().Context); + + Assert.Throws(() => conversion.AddPage(0, text: "a")); + Assert.Throws(() => new VerifySettings().PagesToInclude(0)); + } + + [Fact] + public void PageNames() + { + Assert.Equal("page_0001", PagedConversion.PageName(1)); + Assert.Equal("page_0042", PagedConversion.PageName(42)); + Assert.Equal("page_12345", PagedConversion.PageName(12345)); + } + + /// + /// Some libraries give an empty string for a page with no text. That is a page with no text: + /// nothing in the info for it, and no file for it under PerPage. + /// + [Fact] + public void EmptyTextIsNoText() + { + var settings = new VerifySettings(); + settings.PageText(PageTextPlacement.PerPage); + var perPage = new PagedConversion(settings.Context); + perPage.AddPage(1, text: ""); + perPage.Text(""); + Assert.Empty(perPage.Build().Targets); + + var inInfo = new PagedConversion(new VerifySettings().Context); + inInfo.AddPage(1, text: ""); + inInfo.Text(""); + Assert.Null(inInfo.Build().Info); + } + + /// + /// A converter that asks for the pages and enumerates them later, or not at all, has still + /// said how many the document has. + /// + [Fact] + public void AskingForThePagesRecordsTheCount() + { + var conversion = new PagedConversion(new VerifySettings().Context); + + _ = conversion.Pages(3); + + Assert.Equal(3, conversion.PageCount); + } + + /// + /// Pages are written in page order however the converter came by them. + /// + [Fact] + public void PagesAreOrderedByNumber() + { + var conversion = new PagedConversion(new VerifySettings().Context); + conversion.AddPage(2, new MemoryStream([2])); + conversion.AddPage(1, new MemoryStream([1])); + + var result = conversion.Build(); + + Assert.Null(result.Source); + Assert.Equal( + ["page_0001", "page_0002"], + result.Targets.Select(_ => _.Name)); + } + + /// + /// What the converter is told to skip is read from the settings of the verification it is + /// converting for. + /// + [Fact] + public void TheSettingsOfTheVerificationDecideWhatIsIncluded() + { + var settings = new VerifySettings(); + var conversion = new PagedConversion(settings.Context); + Assert.True(conversion.IncludeImages); + Assert.True(conversion.IncludeText); + Assert.True(conversion.IsPageIncluded(7)); + Assert.Equal([1, 2, 3], conversion.Pages(3)); + + settings.ExcludeDerivedTargets("png"); + settings.PageText(PageTextPlacement.None); + settings.PagesToInclude(2); + Assert.False(conversion.IncludeImages); + Assert.False(conversion.IncludeText); + Assert.False(conversion.IsPageIncluded(3)); + Assert.Equal([1, 2], conversion.Pages(3)); + Assert.Equal(3, conversion.PageCount); + + // A file of text for each page is a derived target like any other, so it can be excluded + settings.PageText(PageTextPlacement.PerPage); + Assert.True(conversion.IncludeText); + settings.ExcludeDerivedTargets("txt"); + Assert.False(conversion.IncludeText); + + // And an image of some other extension is not what was excluded + conversion.ImageExtension = "jpg"; + Assert.True(conversion.IncludeImages); + } + + static List Names(VerifyResult result, string method) => + result.Files + .Select(Path.GetFileName) + .Select(_ => _!.Replace($"{nameof(PagedConversionTests)}.{method}", "Test")) + .OrderBy(_ => _, StringComparer.Ordinal) + .ToList(); + + static string Info(VerifyResult result) => + File.ReadAllText( + result.Files.Single(_ => !Path.GetFileName(_).Contains('#') && _.EndsWith(".verified.txt")), + Encoding.UTF8); +} diff --git a/src/Verify.Tests/Converters/SourceDerivedTests.cs b/src/Verify.Tests/Converters/SourceDerivedTests.cs new file mode 100644 index 0000000000..606bfe2b92 --- /dev/null +++ b/src/Verify.Tests/Converters/SourceDerivedTests.cs @@ -0,0 +1,594 @@ +// A conversion that tells its source from what it derived from it: how the targets are named, what +// is converted again, how they are compared and what an exclusion applies to. What reaches a diff +// tool is in StaticSettingsTests, since that is observed through a static seam. + +using System.Security; + +public class SourceDerivedTests +{ + [ModuleInitializer] + public static void Init() + { + // The shape the document plugins have: the document, and a page of it drawn and read + VerifierSettings.RegisterStreamConverter("sddoc", (_, stream, _) => Document("sddoc", stream)); + + // A container. Each attachment is a document, converted in its turn + VerifierSettings.RegisterStreamConverter( + "sdmail", + (_, stream, _) => + new( + null, + new Target("sdmail", stream), + [ + new("sddoc", Stream("first attachment"), "Attachment1"), + new("sddoc", Stream("second attachment"), "Attachment2") + ])); + + // Gives back its document in a newer format, one that has a converter of its own + VerifierSettings.RegisterStreamConverter( + "sdlegacy", + (_, _, _) => + new( + null, + new Target("sdmodern", Stream("the document, saved as the newer format")), + [new("txt", "derived from the legacy document", "text")])); + + // The same, by a converter that does not tell a source from what it derived + VerifierSettings.RegisterStreamConverter( + "sdouter", + (_, _, _) => + new( + null, + [new("sdmodern", Stream("not for conversion"), null, performConversion: false)])); + + VerifierSettings.RegisterStreamConverter("sdmodern", NeverConverted); + + // A document and a page of it that draws the same whatever the document holds, so that + // the page differs only when its verified file says so + VerifierSettings.RegisterStreamConverter( + "sdbypass", + (_, stream, _) => + { + var content = new MemoryStream(); + stream.CopyTo(content); + return new( + null, + new Target("sdbypass", content), + [new("sdpage", Stream("the page, drawn"), "page_0001")]); + }); + + VerifierSettings.RegisterStreamConverter( + "sdcheck", + (_, stream, context) => + { + renderedPage = !context.IsDerivedTargetExcluded("sdpage"); + List derived = []; + if (renderedPage) + { + derived.Add(new("sdpage", Stream("the page, drawn"), "page_0001")); + } + + return new(null, new Target("sdcheck", stream), derived); + }); + + // Asks before producing anything, so with everything excluded it returns nothing at all + VerifierSettings.RegisterStreamConverter( + "sdskip", + (_, stream, context) => + { + Target? source = null; + if (!context.IsTargetExcluded("sdskip")) + { + source = new("sdskip", Stream("the document")); + } + + List derived = []; + if (!context.IsDerivedTargetExcluded("sdpage")) + { + derived.Add(new("sdpage", Stream("the page, drawn"), "page_0001")); + } + + return new(null, source, derived); + }); + + // Derives a target that a converter of the older kind then converts in its turn + VerifierSettings.RegisterStreamConverter( + "sdchain", + (_, _, _) => new(null, null, [new("sdlink", Stream("to be converted again"), "link")])); + VerifierSettings.RegisterStreamConverter( + "sdlink", + (name, _, _) => new(null, [new("sdpage", Stream("the page, drawn"), name)])); + + VerifierSettings.RegisterFileConverter( + (document, _) => + new( + "the typed info", + new Target("sddoc", Stream(document.Content)), + [new("sdpage", Stream($"page of {document.Content}"), "page_0001")])); + } + + static bool renderedPage; + + static ConversionResult NeverConverted(string? name, Stream stream, IReadOnlyDictionary context) => + throw new("A source, and a target that opted out, are not converted again."); + + record TypedDocument(string Content); + + static ConversionResult Document(string extension, Stream stream) + { + using var reader = new StreamReader(stream); + var content = reader.ReadToEnd(); + return new( + $"info of {content}", + new Target(extension, Stream(content)), + [ + new("sdpage", Stream($"page of {content}"), "page_0001"), + new("txt", $"text of {content}", "page_0001") + ]); + } + + static MemoryStream Stream(string content) => + new(Encoding.UTF8.GetBytes(content)); + + [Fact] + public async Task ADerivedTargetKeepsTheNameItsConverterGaveIt() + { + using var temp = new TempDirectory(); + + var result = await Verify(Stream("the document"), "sddoc") + .UseDirectory(temp) + .AutoVerify() + .DisableDiff(); + + Assert.Equal( + [ + "Test#page_0001.verified.sdpage", + "Test#page_0001.verified.txt", + "Test.verified.sddoc", + "Test.verified.txt" + ], + Names(result, nameof(ADerivedTargetKeepsTheNameItsConverterGaveIt))); + } + + /// + /// The converter of an attachment is passed the attachment's name and ignores it. Its document + /// takes that name, what it derived is named after it, and so is its info. + /// + [Fact] + public async Task NamesAreRelativeToTheTargetThatWasConverted() + { + using var temp = new TempDirectory(); + List attachments = + [ + new("sddoc", Stream("first"), "Attachment1"), + new("sddoc", Stream("second"), "Attachment2") + ]; + + var result = await Verify(attachments) + .UseDirectory(temp) + .AutoVerify() + .DisableDiff(); + + Assert.Equal( + [ + "Test#Attachment1.page_0001.verified.sdpage", + "Test#Attachment1.page_0001.verified.txt", + "Test#Attachment1.verified.sddoc", + "Test#Attachment1.verified.txt", + "Test#Attachment2.page_0001.verified.sdpage", + "Test#Attachment2.page_0001.verified.txt", + "Test#Attachment2.verified.sddoc", + "Test#Attachment2.verified.txt" + ], + Names(result, nameof(NamesAreRelativeToTheTargetThatWasConverted))); + } + + /// + /// The same through a container that is itself converted, where the infos of every conversion + /// are gathered into the one info of the verification. + /// + [Fact] + public async Task ADocumentInsideAContainerIsNamedAfterItsPlaceInIt() + { + using var temp = new TempDirectory(); + + var result = await Verify(Stream("the mail"), "sdmail") + .UseDirectory(temp) + .AutoVerify() + .DisableDiff(); + + Assert.Equal( + [ + "Test#Attachment1.page_0001.verified.sdpage", + "Test#Attachment1.page_0001.verified.txt", + "Test#Attachment1.verified.sddoc", + "Test#Attachment2.page_0001.verified.sdpage", + "Test#Attachment2.page_0001.verified.txt", + "Test#Attachment2.verified.sddoc", + "Test.verified.sdmail", + "Test.verified.txt" + ], + Names(result, nameof(ADocumentInsideAContainerIsNamedAfterItsPlaceInIt))); + } + + /// + /// A document given back in another format is the document, not something to convert: the + /// converter registered for that format throws. + /// + [Fact] + public async Task ASourceIsNotConvertedAgain() + { + using var temp = new TempDirectory(); + + var result = await Verify(Stream("the legacy document"), "sdlegacy") + .UseDirectory(temp) + .AutoVerify() + .DisableDiff(); + + Assert.Equal( + [ + "Test#text.verified.txt", + "Test.verified.sdmodern" + ], + Names(result, nameof(ASourceIsNotConvertedAgain))); + } + + /// + /// What a target passed to a verification could always ask for, and a target a converter + /// returned could not: the flag was not read for those. + /// + [Fact] + public async Task ATargetAConverterReturnedCanOptOutOfConversion() + { + using var temp = new TempDirectory(); + + var result = await Verify(Stream("the outer document"), "sdouter") + .UseDirectory(temp) + .AutoVerify() + .DisableDiff(); + + Assert.Equal( + ["Test.verified.sdmodern"], + Names(result, nameof(ATargetAConverterReturnedCanOptOutOfConversion))); + } + + [Fact] + public async Task ATypedConverterNamesItsSourceTheSameWay() + { + using var temp = new TempDirectory(); + + // The source is a format with a stream converter of its own, which would add a text file + // of the page were it run + var result = await Verify(new TypedDocument("typed")) + .UseDirectory(temp) + .AutoVerify() + .DisableDiff(); + + Assert.Equal( + [ + "Test#page_0001.verified.sdpage", + "Test.verified.sddoc", + "Test.verified.txt" + ], + Names(result, nameof(ATypedConverterNamesItsSourceTheSameWay))); + } + + static int maskedCount; + + // A comparer that masks every difference by always reporting equal + static Task Masking(Stream received, Stream verified, IReadOnlyDictionary context) + { + Interlocked.Increment(ref maskedCount); + return Task.FromResult(CompareResult.Equal); + } + + /// + /// With no flag set by the converter. A page of a document that has not changed is given to + /// its comparer, and a page of one that has is compared exactly. + /// + /// + /// + /// + /// + /// + /// + /// + /// + [Fact] + public async Task ADifferingSourceHasItsDerivedTargetsComparedExactly() + { + using var temp = new TempDirectory(); + var prefix = Path.Combine(temp, $"{nameof(SourceDerivedTests)}.{nameof(ADifferingSourceHasItsDerivedTargetsComparedExactly)}"); + await File.WriteAllTextAsync($"{prefix}.verified.sdbypass", "the document"); + await File.WriteAllTextAsync($"{prefix}#page_0001.verified.sdpage", "a page the comparer would let through"); + + maskedCount = 0; + var equal = await Verify(Stream("the document"), "sdbypass") + .UseDirectory(temp) + .UseStreamComparer(Masking, "sdpage") + .DisableRequireUniquePrefix() + .DisableDiff(); + Assert.Equal(1, maskedCount); + Assert.Equal(2, equal.Files.Count()); + + // Nothing but the document changed, and no text target failed ahead of the page + maskedCount = 0; + var exception = await Assert.ThrowsAsync( + () => Verify(Stream("the document, changed"), "sdbypass") + .UseDirectory(temp) + .UseStreamComparer(Masking, "sdpage") + .DisableRequireUniquePrefix() + .DisableDiff()); + Assert.Equal(0, maskedCount); + Assert.Contains("#page_0001", exception.Message); + } + + /// + /// A text target that fails makes the binary targets after it skip their comparers, which was + /// the only sign there was that a document had changed. With the document itself compared, + /// and equal, a change to its info says nothing of its pages. + /// + [Fact] + public async Task AnInfoThatDiffersLeavesThePagesOfAnUnchangedSourceToTheirComparers() + { + using var temp = new TempDirectory(); + var prefix = Path.Combine(temp, $"{nameof(SourceDerivedTests)}.{nameof(AnInfoThatDiffersLeavesThePagesOfAnUnchangedSourceToTheirComparers)}"); + await File.WriteAllTextAsync($"{prefix}.verified.txt", "an info in the shape it used to have"); + await File.WriteAllTextAsync($"{prefix}.verified.sddoc", "the document"); + await File.WriteAllTextAsync($"{prefix}#page_0001.verified.sdpage", "a page the comparer would let through"); + await File.WriteAllTextAsync($"{prefix}#page_0001.verified.txt", "text of the document"); + + maskedCount = 0; + var exception = await Assert.ThrowsAsync( + () => Verify(Stream("the document"), "sddoc") + .UseDirectory(temp) + .UseStreamComparer(Masking, "sdpage") + .DisableDiff()); + + Assert.Equal(1, maskedCount); + var notEqual = NotEqual(exception.Message); + Assert.Contains(".verified.txt", notEqual); + Assert.DoesNotContain("#page_0001", notEqual); + } + + /// + /// One document differing says nothing of the pages of another, which the flag this replaces + /// could not express: it switched the comparers off for every target that followed. + /// + [Fact] + public async Task ADifferingSourceLeavesTheTargetsOfAnotherToTheirComparers() + { + using var temp = new TempDirectory(); + var prefix = Path.Combine(temp, $"{nameof(SourceDerivedTests)}.{nameof(ADifferingSourceLeavesTheTargetsOfAnotherToTheirComparers)}"); + foreach (var attachment in new[] {"Attachment1", "Attachment2"}) + { + await File.WriteAllTextAsync($"{prefix}#{attachment}.verified.sdbypass", attachment); + await File.WriteAllTextAsync($"{prefix}#{attachment}.page_0001.verified.sdpage", "a page the comparer would let through"); + } + + List attachments = + [ + new("sdbypass", Stream("Attachment1, changed"), "Attachment1"), + new("sdbypass", Stream("Attachment2"), "Attachment2") + ]; + + maskedCount = 0; + var exception = await Assert.ThrowsAsync( + () => Verify(attachments) + .UseDirectory(temp) + .UseStreamComparer(Masking, "sdpage") + .DisableDiff()); + + // The second attachment's page went to the comparer, and only that one + Assert.Equal(1, maskedCount); + Assert.Contains("#Attachment1.page_0001", exception.Message); + Assert.DoesNotContain("#Attachment2.page_0001", NotEqual(exception.Message)); + } + + /// + /// The info is the first target and the document the second, so going through them in order + /// compared the info before anything was known of the document. + /// + [Fact] + public async Task TheInfoOfADifferingSourceIsComparedExactlyToo() + { + using var temp = new TempDirectory(); + var prefix = Path.Combine(temp, $"{nameof(SourceDerivedTests)}.{nameof(TheInfoOfADifferingSourceIsComparedExactlyToo)}"); + await File.WriteAllTextAsync($"{prefix}.verified.txt", "an info the comparer would let through"); + await File.WriteAllTextAsync($"{prefix}.verified.sddoc", "the document"); + await File.WriteAllTextAsync($"{prefix}#page_0001.verified.sdpage", "page of the document, changed"); + await File.WriteAllTextAsync($"{prefix}#page_0001.verified.txt", "text of the document, changed"); + + var compared = 0; + var exception = await Assert.ThrowsAsync( + () => Verify(Stream("the document, changed"), "sddoc") + .UseDirectory(temp) + .UseStringComparer( + (_, _, _) => + { + compared++; + return Task.FromResult(CompareResult.Equal); + }) + .DisableDiff()); + + Assert.Equal(0, compared); + Assert.Contains("TheInfoOfADifferingSourceIsComparedExactlyToo.verified.txt", NotEqual(exception.Message)); + } + + [Fact] + public async Task ADerivedTargetCanBeExcluded() + { + using var temp = new TempDirectory(); + + var result = await Verify(Stream("the document"), "sddoc") + .UseDirectory(temp) + .ExcludeDerivedTargets("sdpage") + .AutoVerify() + .DisableDiff(); + + Assert.Equal( + [ + "Test#page_0001.verified.txt", + "Test.verified.sddoc", + "Test.verified.txt" + ], + Names(result, nameof(ADerivedTargetCanBeExcluded))); + } + + /// + /// Which is the difference from ExcludeTargets: the extension is only excluded where a + /// converter derived the target. The document, the info and a target passed in stay. + /// + [Fact] + public async Task ExcludingADerivedExtensionLeavesTheSameExtensionElsewhere() + { + using var temp = new TempDirectory(); + List targets = [new("sdpage", Stream("a page passed in as itself"))]; + + var passed = await Verify(targets) + .UseDirectory(temp) + .ExcludeDerivedTargets("sdpage") + .AutoVerify() + .DisableDiff(); + Assert.Equal( + ["Test.verified.sdpage"], + Names(passed, nameof(ExcludingADerivedExtensionLeavesTheSameExtensionElsewhere))); + + var document = await Verify(Stream("the document"), "sddoc") + .UseDirectory(temp) + .ExcludeDerivedTargets("sddoc", "txt") + .DisableRequireUniquePrefix() + .AutoVerify() + .DisableDiff(); + Assert.Contains( + "Test.verified.sddoc", + Names(document, nameof(ExcludingADerivedExtensionLeavesTheSameExtensionElsewhere))); + Assert.Contains( + "Test.verified.txt", + Names(document, nameof(ExcludingADerivedExtensionLeavesTheSameExtensionElsewhere))); + Assert.DoesNotContain( + "Test#page_0001.verified.txt", + Names(document, nameof(ExcludingADerivedExtensionLeavesTheSameExtensionElsewhere))); + } + + /// + /// The converter observes the exclusion and never draws the page, for either way of excluding it. + /// + [Fact] + public async Task AConverterCanAskWhetherADerivedTargetIsExcluded() + { + using var temp = new TempDirectory(); + + renderedPage = false; + await Verify(Stream("the document"), "sdcheck") + .UseDirectory(temp) + .DisableRequireUniquePrefix() + .AutoVerify() + .DisableDiff(); + Assert.True(renderedPage); + + await Verify(Stream("the document"), "sdcheck") + .UseDirectory(temp) + .ExcludeDerivedTargets("sdpage") + .DisableRequireUniquePrefix() + .AutoVerify() + .DisableDiff(); + Assert.False(renderedPage); + + renderedPage = true; + await Verify(Stream("the document"), "sdcheck") + .UseDirectory(temp) + .ExcludeTargets("sdpage") + .DisableRequireUniquePrefix() + .AutoVerify() + .DisableDiff(); + Assert.False(renderedPage); + } + + [Fact] + public async Task ExcludingEveryTargetThrows() + { + var exception = await Assert.ThrowsAsync( + () => Verify(Stream("the document"), "sdcheck") + .ExcludeTargets("sdcheck") + .ExcludeDerivedTargets("sdpage") + .DisableDiff()); + Assert.Contains("All targets have been excluded", exception.Message); + } + + /// + /// A converter that is told what is excluded produces none of it, so there is nothing left + /// for the exclusion to remove, and nothing to say the verification verified nothing. + /// + [Fact] + public async Task ExcludingEveryTargetThrowsWhenTheConverterLeftThemOutItself() + { + var exception = await Assert.ThrowsAsync( + () => Verify(Stream("the document"), "sdskip") + .ExcludeTargets("sdskip") + .ExcludeDerivedTargets("sdpage") + .DisableDiff()); + Assert.Contains("All targets have been excluded", exception.Message); + } + + /// + /// What a converter of the older kind makes of a derived target is derived too, including + /// where the document it all came from was left out. + /// + [Fact] + public async Task WhatIsConvertedFromADerivedTargetIsDerived() + { + using var temp = new TempDirectory(); + + var kept = await Verify(Stream("the document"), "sdchain") + .UseDirectory(temp) + .AutoVerify() + .DisableDiff(); + Assert.Equal( + ["Test#link.verified.sdpage"], + Names(kept, nameof(WhatIsConvertedFromADerivedTargetIsDerived))); + + var exception = await Assert.ThrowsAsync( + () => Verify(Stream("the document"), "sdchain") + .UseDirectory(temp) + .ExcludeDerivedTargets("sdpage") + .DisableRequireUniquePrefix() + .DisableDiff()); + Assert.Contains("All targets have been excluded", exception.Message); + } + + [Fact] + public void NoExtensionsThrows() => + Assert.Throws(() => new VerifySettings().ExcludeDerivedTargets()); + + // The verified file names of a result, less the test they belong to, in a fixed order + static List Names(VerifyResult result, string method) => + result.Files + .Select(Path.GetFileName) + .Select(_ => _!.Replace($"{nameof(SourceDerivedTests)}.{method}", "Test")) + .OrderBy(_ => _, StringComparer.Ordinal) + .ToList(); + + // The part of an exception message that lists the files that differed + static string NotEqual(string message) + { + var start = message.IndexOf("NotEqual:", StringComparison.Ordinal); + if (start < 0) + { + return ""; + } + + var end = message.Length; + foreach (var section in new[] {"\nDelete:", "\nEqual:", "\nFileContent:"}) + { + var index = message.IndexOf(section, start, StringComparison.Ordinal); + if (index >= 0 && + index < end) + { + end = index; + } + } + + return message.Substring(start, end - start); + } +} diff --git a/src/Verify.Tests/InnerVerifyTests/InnerVerifyTests.cs b/src/Verify.Tests/InnerVerifyTests/InnerVerifyTests.cs index 3cbcc5a240..1f4c2d04de 100644 --- a/src/Verify.Tests/InnerVerifyTests/InnerVerifyTests.cs +++ b/src/Verify.Tests/InnerVerifyTests/InnerVerifyTests.cs @@ -83,4 +83,35 @@ public async Task Split() using var verifier = new InnerVerifier(targetDirectory, "split"); await verifier.VerifyFile(splitFilePath); } + + /// + /// A named target that is the only one of its name was written to the file an unnamed one + /// has, so two of them, each page of a document, went to the same file. + /// + [Fact] + public async Task NamedTargetsAreNamed() + { + using var temp = new TempDirectory(); + var settings = new VerifySettings(); + settings.AutoVerify(); + settings.DisableDiff(); + using var verifier = new InnerVerifier(temp, "named", settings); + List targets = + [ + new("txt", "the first page", "page_0001"), + new("txt", "the second page", "page_0002") + ]; + + var result = await verifier.Verify(targets); + + Assert.Equal( + [ + "named#page_0001.verified.txt", + "named#page_0002.verified.txt" + ], + result.Files.Select(Path.GetFileName)); + Assert.Equal( + "the second page", + await File.ReadAllTextAsync(Path.Combine(temp.Path, "named#page_0002.verified.txt"), Encoding.UTF8)); + } } \ No newline at end of file diff --git a/src/Verify.Tests/Snippets/PagedDocumentSnippets.cs b/src/Verify.Tests/Snippets/PagedDocumentSnippets.cs new file mode 100644 index 0000000000..7f36c1cb1c --- /dev/null +++ b/src/Verify.Tests/Snippets/PagedDocumentSnippets.cs @@ -0,0 +1,109 @@ +#if DEBUG +// ReSharper disable UnusedMember.Local +// ReSharper disable UnusedParameter.Local +#pragma warning disable IDE0060 + +public class PagedDocumentSnippets +{ + static Task PageText(Stream pdf) => + + #region PageTextPerPage + + Verify(pdf, "pdf") + .PageText(PageTextPlacement.PerPage); + + #endregion + + static Task NoPageText(Stream pdf) => + + #region PageTextNone + + Verify(pdf, "pdf") + .PageText(PageTextPlacement.None); + + #endregion + + static Task PagesToIncludeCount(Stream pdf) => + + #region PagesToIncludeCount + + Verify(pdf, "pdf") + .PagesToInclude(2); + + #endregion + + static Task PagesToIncludeDelegate(Stream pdf) => + + #region PagesToIncludeDelegate + + Verify(pdf, "pdf") + .PagesToInclude(pageNumber => pageNumber is 1 or 5); + + #endregion + + static Task ExcludeDerivedTargets(Stream pdf) => + + #region ExcludeDerivedTargets + + Verify(pdf, "pdf") + .ExcludeDerivedTargets("png"); + + #endregion + + #region SourceAndDerivedTargets + + // A converter of a workbook: the workbook is the source, and a csv of each sheet is derived + static ConversionResult ConvertWorkbook(string? name, Stream stream, IReadOnlyDictionary context) + { + var workbook = Workbook.Load(stream); + + var sheets = new List(); + foreach (var sheet in workbook.Sheets) + { + // Named by what it is. The name of the target being converted is added by Verify + sheets.Add(new("csv", sheet.ToCsv(), sheet.Name)); + } + + Target? source = null; + if (!context.IsTargetExcluded("xlsx")) + { + source = new("xlsx", workbook.Save()); + } + + return new( + info: new + { + workbook.Author + }, + source, + derived: sheets); + } + + #endregion + + class Workbook + { + public static Workbook Load(Stream stream) => + new(); + + // ReSharper disable once MemberCanBeMadeStatic.Local + public string Author => ""; + public List Sheets { get; } = []; + + // ReSharper disable once MemberCanBeMadeStatic.Local + public Stream Save() => + new MemoryStream(); + } + + class Sheet + { + // ReSharper disable once MemberCanBeMadeStatic.Local + public string Name => ""; + + // ReSharper disable once MemberCanBeMadeStatic.Local + public string ToCsv() => + ""; + } +} + +#endif diff --git a/src/Verify/ReceivedMap.cs b/src/Verify/ReceivedMap.cs index 469c57a538..d1576de73c 100644 --- a/src/Verify/ReceivedMap.cs +++ b/src/Verify/ReceivedMap.cs @@ -14,12 +14,18 @@ /// The file name is derived from the received path, so a re run overwrites the same map instead of /// accumulating. Stale maps, for example from a deleted test, are never read, since tooling looks up /// maps by the received files that actually exist. They are removed whenever obj is cleaned. +/// +/// A map is a line each: the received path, the verified path, and, for a file a converter derived +/// from a document that is itself pending, the received path of that document. The third line is +/// what lets tooling treat a document and its pages as one change to accept. It is absent for a +/// file that stands alone, and a reader from before it existed reads the first two lines of a map +/// that has it. /// static class ReceivedMap { const string directoryName = "VerifyReceived"; - public static void Write(in FilePair file) + public static void Write(in FilePair file, string? source) { if (BuildServerDetector.Detected) { @@ -40,7 +46,13 @@ public static void Write(in FilePair file) var directory = Path.Combine(intermediate, directoryName); Directory.CreateDirectory(directory); var path = Path.Combine(directory, $"{Fnv1a.Hash(file.ReceivedPath)}.txt"); - File.WriteAllText(path, $"{file.ReceivedPath}{Environment.NewLine}{file.VerifiedPath}"); + var content = $"{file.ReceivedPath}{Environment.NewLine}{file.VerifiedPath}"; + if (source is not null) + { + content = $"{content}{Environment.NewLine}{source}"; + } + + File.WriteAllText(path, content); } catch { diff --git a/src/Verify/Serialization/VerifierSettings.cs b/src/Verify/Serialization/VerifierSettings.cs index 4dc85f98a4..319729de1f 100644 --- a/src/Verify/Serialization/VerifierSettings.cs +++ b/src/Verify/Serialization/VerifierSettings.cs @@ -174,6 +174,9 @@ internal static void Reset() encoding = new UTF8Encoding(true, true); addAttachments = true; excludedTargets = null; + excludedDerivedTargets = null; + pageText = null; + includePage = null; GlobalScrubbers.Clear(); ExtensionMappedGlobalScrubbers.Clear(); GlobalSpanScrubbers.Clear(); diff --git a/src/Verify/Splitters/ConversionResult.cs b/src/Verify/Splitters/ConversionResult.cs index 9efc3ce682..e1ba7a3c77 100644 --- a/src/Verify/Splitters/ConversionResult.cs +++ b/src/Verify/Splitters/ConversionResult.cs @@ -1,11 +1,24 @@ -namespace VerifyTests; +namespace VerifyTests; public readonly struct ConversionResult { public object? Info { get; } + /// + /// Every target of the conversion. Where a was named it is the first. + /// public IEnumerable Targets { get; } + /// + /// The document the other were derived from, where the conversion named + /// one. See . + /// + public Target? Source { get; } + + // Set by the constructor that tells a source from what was derived from it, a null source + // included. What says the conversion follows the naming of that constructor. + internal bool IsDerivation { get; } + public Func? Cleanup { get; } public ConversionResult(object? info, IEnumerable targets, Func? cleanup = null) @@ -15,6 +28,59 @@ public ConversionResult(object? info, IEnumerable targets, Func? c Cleanup = cleanup; } + /// + /// A conversion of a document into the document itself, the , and + /// the targets computed from it, the : a rendered image or the + /// text of each page, a csv for each sheet. + /// + /// + /// Saying which is which is what lets the rest be done once, here, rather than by each + /// converter: + /// + /// + /// The source is compared first, and when it differs its derived targets skip their + /// registered comparers and fall back to exact comparison. There is no need for + /// . + /// + /// + /// The source is never converted again, whatever its extension. + /// + /// + /// Names are relative to the target that was converted. A derived target named + /// page_0001, from a converted target named Attachment1, is + /// Attachment1.page_0001, and the source and the info take Attachment1. So a + /// converter ignores the name it is passed. + /// + /// + /// A diff tool is told that the derived files came from the source. One that shows the + /// source as a document, with its pages, accepts them together with it. + /// + /// + /// + /// Written to the info file. Null for none. + /// + /// The document. Null where it is not wanted as a target, for example when + /// context.IsTargetExcluded says its extension is excluded, which leaves the derived + /// targets standing alone. + /// + /// The targets computed from the document. + /// Run once the verification no longer needs the targets. + public ConversionResult(object? info, Target? source, IEnumerable derived, Func? cleanup = null) + { + Info = info; + Cleanup = cleanup; + Source = source; + IsDerivation = true; + if (source is { } value) + { + Targets = [value, ..derived]; + } + else + { + Targets = derived; + } + } + public ConversionResult(object? info, string extension, Stream stream, Func? cleanup = null) { Ensure.NotNullOrEmpty(extension); @@ -38,4 +104,4 @@ public ConversionResult(object? info, string extension, StringBuilder data, Func Cleanup = cleanup; Targets = [new(extension, data)]; } -} \ No newline at end of file +} diff --git a/src/Verify/Splitters/ConversionToken.cs b/src/Verify/Splitters/ConversionToken.cs new file mode 100644 index 0000000000..3a1be8cc8b --- /dev/null +++ b/src/Verify/Splitters/ConversionToken.cs @@ -0,0 +1,14 @@ +/// +/// One conversion that named a source, and so the identity shared by that source and every target +/// the conversion derived from it. +/// +/// +/// Compared by reference: two conversions of two attachments are two tokens, however alike their +/// targets are. is the conversion that produced the target this one +/// converted, which is how a page of a pdf that was itself rendered from a docx is traced back to +/// the docx. +/// +sealed class ConversionToken(ConversionToken? parent) +{ + public ConversionToken? Parent { get; } = parent; +} diff --git a/src/Verify/Splitters/PagedConversion.cs b/src/Verify/Splitters/PagedConversion.cs new file mode 100644 index 0000000000..9d089c0828 --- /dev/null +++ b/src/Verify/Splitters/PagedConversion.cs @@ -0,0 +1,270 @@ +namespace VerifyTests; + +/// +/// Builds the of a converter for a paged document: the document +/// as the source, and an image and the text of each page as what was derived from it. +/// +/// +/// What every such converter would otherwise decide for itself is decided here, from the settings +/// of the verification: how the page files are named, where the text goes +/// (), which pages are verified (PagesToInclude), and which +/// derived targets are left out (ExcludeDerivedTargets). +/// +/// A page is the unit whatever the document calls it: a page of a pdf, a slide of a +/// presentation, a frame of an image. Page numbers are 1 based. +/// +/// +public sealed class PagedConversion(IReadOnlyDictionary context) +{ + Target? source; + string? text; + List pages = []; + List derived = []; + + /// + /// The extension of the image of a page. png by default. + /// + public string ImageExtension { get; set; } = "png"; + + /// + /// The extension of the text files written under . + /// txt by default. A converter that reads a document as markdown sets md. + /// + public string TextExtension { get; set; } = "txt"; + + /// + /// What the converter says about the document as a whole. Written to the info file. + /// + public object? Info { get; set; } + + /// + /// The number of pages in the document, counting those that are not verified. Written to the + /// info file. Set by , and by + /// when nothing else has set it. + /// + public int? PageCount { get; set; } + + /// + /// Whether the images of pages are verified. False when their extension is excluded, in which + /// case a converter can skip rendering. + /// + public bool IncludeImages => !context.IsDerivedTargetExcluded(ImageExtension); + + /// + /// Whether the text of the document is verified. False under + /// , and under + /// when the extension of the text files is excluded, in which case a converter can skip + /// reading it. + /// + public bool IncludeText + { + get + { + var placement = context.PageTextPlacement(); + if (placement == PageTextPlacement.None) + { + return false; + } + + if (placement == PageTextPlacement.PerPage) + { + return !context.IsDerivedTargetExcluded(TextExtension); + } + + return true; + } + } + + /// + /// Whether the page with the 1 based is verified. + /// + public bool IsPageIncluded(int number) => + context.IsPageIncluded(number); + + /// + /// The 1 based numbers of the pages to verify, of a document with + /// pages. Records . + /// + public IEnumerable Pages(int pageCount) + { + // Recorded here rather than in the iterator, which runs nothing until it is enumerated + PageCount = pageCount; + return Included(pageCount); + } + + IEnumerable Included(int pageCount) + { + for (var number = 1; number <= pageCount; number++) + { + if (IsPageIncluded(number)) + { + yield return number; + } + } + } + + /// + /// The document itself. Leave it out when context.IsTargetExcluded says its extension + /// is excluded, so that it does not have to be produced. + /// + public void Source(Target document) => + source = document; + + /// + /// A page of the document. + /// + /// The 1 based number of the page. + /// The page rendered. The stream is owned by the verification. + /// The text read from the page. + /// What the converter says about the page. Written to the info file. + public void AddPage(int number, Stream? image = null, string? text = null, object? info = null) + { + if (number < 1) + { + throw new ArgumentOutOfRangeException(nameof(number), number, "Page numbers are 1 based."); + } + + pages.Add(new(number, image, NullIfEmpty(text), info)); + } + + // A page with no text has none, however the library reading it says so. Kept as empty it + // would be an empty member in the info file, or a file for the page with nothing in it + static string? NullIfEmpty(string? text) + { + if (string.IsNullOrEmpty(text)) + { + return null; + } + + return text; + } + + /// + /// The image of every page of the document, in page order, from a renderer that produces them + /// all at once and so cannot skip a page that is not verified. Those are dropped here. + /// + public void AddImages(IEnumerable images) + { + var number = 0; + foreach (var image in images) + { + number++; + if (IsPageIncluded(number)) + { + AddPage(number, image); + } + else + { + image.Dispose(); + } + } + + PageCount ??= number; + } + + /// + public void AddImages(IEnumerable images) => + AddImages(images.Select(Stream (_) => new MemoryStream(_))); + + /// + /// The text of the whole document, from a converter that cannot say which page each part of + /// it is on. + /// + public void Text(string? text) => + this.text = NullIfEmpty(text); + + /// + /// Any other target computed from the document: a csv for each sheet, the styles of a + /// workbook. It has to be named, since the name is what tells it from the other targets. + /// + public void AddDerived(Target target) + { + if (target.Name is null) + { + throw new ArgumentException("A derived target requires a name.", nameof(target)); + } + + derived.Add(target); + } + + /// + /// The name of the files for the page with the 1 based . + /// + public static string PageName(int number) => + $"page_{number:0000}"; + + /// + /// The result to return from the converter. + /// + /// Run once the verification no longer needs the targets. + public ConversionResult Build(Func? cleanup = null) + { + var placement = context.PageTextPlacement(); + var includeImages = IncludeImages; + var includeText = IncludeText; + var targets = new List(); + var infos = new List(); + + if (includeText && + placement == PageTextPlacement.PerPage && + text is not null) + { + targets.Add(new(TextExtension, text, "text")); + } + + foreach (var page in pages.OrderBy(_ => _.Number)) + { + var name = PageName(page.Number); + if (page.Image is { } image) + { + if (includeImages) + { + targets.Add(new(ImageExtension, image, name)); + } + else + { + // Rendered by a converter that did not ask first. The verification owns the + // stream of a target it is given, and this one never becomes a target. + image.Dispose(); + } + } + + string? inInfo = null; + if (includeText && + page.Text is not null) + { + if (placement == PageTextPlacement.PerPage) + { + targets.Add(new(TextExtension, page.Text, name)); + } + else + { + inInfo = page.Text; + } + } + + if (page.Info is not null || + inInfo is not null) + { + infos.Add(new(page.Number, page.Info, inInfo)); + } + } + + targets.AddRange(derived); + + string? textInInfo = null; + if (includeText && + placement == PageTextPlacement.InInfo) + { + textInInfo = text; + } + + return new( + PagedInfo.Build(Info, PageCount, textInInfo, infos), + source, + targets, + cleanup); + } + + record Page(int Number, Stream? Image, string? Text, object? Info); +} diff --git a/src/Verify/Splitters/PagedInfo.cs b/src/Verify/Splitters/PagedInfo.cs new file mode 100644 index 0000000000..08d9115181 --- /dev/null +++ b/src/Verify/Splitters/PagedInfo.cs @@ -0,0 +1,45 @@ +/// +/// The info file of a paged document, in the one shape every converter built on +/// writes: what the converter says of the document, how many pages +/// it has, its text, and what there is to say of each page. +/// +/// +/// Serialized as any object is, so a member that is null is left out, and the settings of the +/// verification - ignored members, scrubbers - apply to it as they do to anything else. +/// +class PagedInfo +{ + public object? Document { get; private init; } + public int? PageCount { get; private init; } + public string? Text { get; private init; } + public IReadOnlyList? Pages { get; private init; } + + /// + /// Null when there is nothing to say, so that no info file is written. + /// + public static PagedInfo? Build(object? document, int? pageCount, string? text, List pages) + { + if (document is null && + pageCount is null && + text is null && + pages.Count == 0) + { + return null; + } + + return new() + { + Document = document, + PageCount = pageCount, + Text = text, + Pages = pages.Count == 0 ? null : pages + }; + } + + public class Page(int number, object? info, string? text) + { + public int Number { get; } = number; + public object? Info { get; } = info; + public string? Text { get; } = text; + } +} diff --git a/src/Verify/Splitters/Settings_ExcludeTargets.cs b/src/Verify/Splitters/Settings_ExcludeTargets.cs index 0990adb194..cd2ce2c628 100644 --- a/src/Verify/Splitters/Settings_ExcludeTargets.cs +++ b/src/Verify/Splitters/Settings_ExcludeTargets.cs @@ -35,7 +35,43 @@ internal static bool IsExcluded(IReadOnlyDictionary context, str internal static bool AnyExcludedTargets(IReadOnlyDictionary context) => excludedTargets is { Count: > 0 } || - context.ContainsKey(VerifySettings.excludedTargetsKey); + excludedDerivedTargets is { Count: > 0 } || + context.ContainsKey(VerifySettings.excludedTargetsKey) || + context.ContainsKey(VerifySettings.excludedDerivedTargetsKey); + + internal static HashSet? excludedDerivedTargets; + + /// + /// Excludes, from all verifications, every target with one of + /// that a converter derived from a document: a rendered image or the text of a page, a csv of + /// a sheet. The document itself, and a target passed to a verification directly, are kept, + /// which is the difference from : excluding png there + /// would also exclude a verified image. + /// Any existing verified file for an excluded derived target is treated as pending deletion. + /// + public static void ExcludeDerivedTargets(params string[] extensions) + { + InnerVerifier.ThrowIfVerifyHasBeenRun(); + excludedDerivedTargets = AddExtensions(excludedDerivedTargets, extensions); + } + + /// + /// Whether a derived target with will be excluded from the + /// current verification, via ExcludeDerivedTargets or ExcludeTargets, global or + /// per-verification. A converter can call this on its context to skip producing an + /// expensive derived target (eg rendering each page) that would otherwise be dropped. + /// + public static bool IsDerivedTargetExcluded(this IReadOnlyDictionary context, string extension) + { + Guards.AgainstBadExtension(extension); + return IsDerivedExcluded(context, extension); + } + + internal static bool IsDerivedExcluded(IReadOnlyDictionary context, string extension) => + IsExcluded(context, extension) || + excludedDerivedTargets?.Contains(extension) == true || + (context.TryGetValue(VerifySettings.excludedDerivedTargetsKey, out var value) && + ((HashSet) value).Contains(extension)); // 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. @@ -76,6 +112,21 @@ public void ExcludeTargets(params string[] extensions) var existing = Context.TryGetValue(excludedTargetsKey, out var value) ? (HashSet) value : null; Context[excludedTargetsKey] = VerifierSettings.AddExtensions(existing, extensions); } + + internal const string excludedDerivedTargetsKey = "Verify.ExcludeDerivedTargets"; + + /// + /// Excludes, from the current verification, every target with one of + /// that a converter derived from a document: a rendered image + /// or the text of a page, a csv of a sheet. The document itself, and a target passed to the + /// verification directly, are kept. + /// Any existing verified file for an excluded derived target is treated as pending deletion. + /// + public void ExcludeDerivedTargets(params string[] extensions) + { + var existing = Context.TryGetValue(excludedDerivedTargetsKey, out var value) ? (HashSet) value : null; + Context[excludedDerivedTargetsKey] = VerifierSettings.AddExtensions(existing, extensions); + } } public partial class SettingsTask @@ -87,4 +138,12 @@ public SettingsTask ExcludeTargets(params string[] extensions) CurrentSettings.ExcludeTargets(extensions); return this; } + + /// + [Pure] + public SettingsTask ExcludeDerivedTargets(params string[] extensions) + { + CurrentSettings.ExcludeDerivedTargets(extensions); + return this; + } } diff --git a/src/Verify/Splitters/Settings_Pages.cs b/src/Verify/Splitters/Settings_Pages.cs new file mode 100644 index 0000000000..2a7a2c5ba3 --- /dev/null +++ b/src/Verify/Splitters/Settings_Pages.cs @@ -0,0 +1,163 @@ +namespace VerifyTests; + +/// +/// Where the converter of a paged document puts the text it reads from the document. +/// +public enum PageTextPlacement +{ + /// + /// In the info file, with whatever else the converter says about the document. The default. + /// + InInfo, + + /// + /// In a text file for each page, named as the image of that page is. + /// + PerPage, + + /// + /// Nowhere. The text is not verified, and a converter can skip reading it. + /// + None +} + +/// +/// Whether the page with the 1 based is verified. +/// +public delegate bool IncludePage(int pageNumber); + +public static partial class VerifierSettings +{ + internal static PageTextPlacement? pageText; + internal static IncludePage? includePage; + + /// + /// Where converters of paged documents put the text of a document, for all verifications. + /// + public static void PageText(PageTextPlacement placement) + { + InnerVerifier.ThrowIfVerifyHasBeenRun(); + pageText = placement; + } + + /// + /// Limits what converters of paged documents verify to the first + /// pages, for all verifications. The document itself is still verified whole. + /// + public static void PagesToInclude(int count) + { + InnerVerifier.ThrowIfVerifyHasBeenRun(); + includePage = FirstPages(count); + } + + /// + /// Limits what converters of paged documents verify to the pages + /// accepts, for all verifications. The document itself is still verified whole. + /// + public static void PagesToInclude(IncludePage include) + { + InnerVerifier.ThrowIfVerifyHasBeenRun(); + includePage = include; + } + + internal static IncludePage FirstPages(int count) + { + if (count < 1) + { + throw new ArgumentOutOfRangeException(nameof(count), count, "At least one page is required."); + } + + return _ => _ <= count; + } + + /// + /// Where the text of a paged document goes in the current verification, via either the global + /// or the per-verification PageText. Read by a converter from its context. + /// + public static PageTextPlacement PageTextPlacement(this IReadOnlyDictionary context) + { + if (context.TryGetValue(VerifySettings.pageTextKey, out var value)) + { + return (PageTextPlacement) value; + } + + return pageText.GetValueOrDefault(); + } + + /// + /// Whether the page with the 1 based is verified in the current + /// verification, via either the global or the per-verification PagesToInclude. A + /// converter can call this on its context to skip rendering a page that would otherwise + /// be dropped. + /// + public static bool IsPageIncluded(this IReadOnlyDictionary context, int pageNumber) + { + if (context.TryGetValue(VerifySettings.includePageKey, out var value)) + { + return ((IncludePage) value)(pageNumber); + } + + if (includePage is null) + { + return true; + } + + return includePage(pageNumber); + } +} + +public partial class VerifySettings +{ + // In Context, as the excluded targets are, so that a converter, which receives Context, can + // read them. + internal const string pageTextKey = "Verify.PageText"; + internal const string includePageKey = "Verify.PagesToInclude"; + + /// + /// Where converters of paged documents put the text of the document, for the current + /// verification. + /// + public void PageText(PageTextPlacement placement) => + Context[pageTextKey] = placement; + + /// + /// Limits what converters of paged documents verify to the first + /// pages, for the current verification. The document itself is still verified whole. + /// + public void PagesToInclude(int count) => + Context[includePageKey] = VerifierSettings.FirstPages(count); + + /// + /// Limits what converters of paged documents verify to the pages + /// accepts, for the current verification. The document itself is still verified whole. + /// + public void PagesToInclude(IncludePage include) => + Context[includePageKey] = include; +} + +public partial class SettingsTask +{ + /// + [Pure] + public SettingsTask PageText(PageTextPlacement placement) + { + CurrentSettings.PageText(placement); + return this; + } + + /// + [Pure] + public SettingsTask PagesToInclude(int count) + { + CurrentSettings.PagesToInclude(count); + return this; + } + + /// + [Pure] + public SettingsTask PagesToInclude(IncludePage include) + { + CurrentSettings.PagesToInclude(include); + return this; + } +} diff --git a/src/Verify/Target.cs b/src/Verify/Target.cs index 16f27d3234..9f0b2ed1e4 100644 --- a/src/Verify/Target.cs +++ b/src/Verify/Target.cs @@ -5,9 +5,27 @@ public readonly struct Target readonly StringBuilder? stringBuilderData; readonly Stream? streamData; public string Extension { get; } - public string? Name { get; } = null; + public string? Name { get; internal init; } = null; public bool PerformConversion { get; } = true; + /// + /// The conversion this target came out of, where that conversion named a source: what ties a + /// derived target to the document it was computed from. Null for a target that stands alone. + /// + internal ConversionToken? Conversion { get; init; } + + /// + /// This target is the source of : the document the other targets of + /// that conversion were derived from. See . + /// + internal bool IsSource { get; init; } + + /// + /// This target was computed from a document by a conversion that told the two apart, whether + /// or not the document is itself a target. What ExcludeDerivedTargets applies to. + /// + internal bool IsDerived { get; init; } + /// /// When true and this target differs from its verified file, all subsequent targets in the /// same verification skip their registered comparers and fall back to exact (binary or string) comparison. diff --git a/src/Verify/Verifier/InnerVerifier.cs b/src/Verify/Verifier/InnerVerifier.cs index 9fe06664b8..b57ec1a4ff 100644 --- a/src/Verify/Verifier/InnerVerifier.cs +++ b/src/Verify/Verifier/InnerVerifier.cs @@ -224,11 +224,24 @@ public InnerVerifier(string directory, string name, VerifySettings? settings = n verifiedFiles = MatchingFileFinder.FindVerified(name, directory); getFileNames = target => - new( + { + if (target.Name is null) + { + return new( + target.Extension, + $"{prefix}.received.{target.Extension}", + $"{prefix}.verified.{target.Extension}", + target.IsString); + } + + // As the file convention names it. Without the name, every named target that is the + // only one of its name was written to the one file: each page of a document + return new( target.Extension, - $"{prefix}.received.{target.Extension}", - $"{prefix}.verified.{target.Extension}", + $"{prefix}#{target.Name}.received.{target.Extension}", + $"{prefix}#{target.Name}.verified.{target.Extension}", target.IsString); + }; getIndexedFileNames = (target, index) => { diff --git a/src/Verify/Verifier/InnerVerifier_Inner.cs b/src/Verify/Verifier/InnerVerifier_Inner.cs index 9c8171b48e..d1380aac97 100644 --- a/src/Verify/Verifier/InnerVerifier_Inner.cs +++ b/src/Verify/Verifier/InnerVerifier_Inner.cs @@ -5,10 +5,19 @@ partial class InnerVerifier Task VerifyInner(IEnumerable targets) => VerifyInner(null, null, targets, true, true); - async Task VerifyInner(object? root, Func? cleanup, IEnumerable targets, bool doExtensionConversion, bool ignoreNullRoot) + /// + /// The conversion is the info of, where it is nothing but what + /// converters said of a source one of them named. The file it is written to is then derived + /// from that source, as the other targets of the conversion are. + /// + /// + /// is nothing but what converters said of a document, one that may + /// not itself be a target. The file it is written to is then never the inline snapshot. + /// + async Task VerifyInner(object? root, Func? cleanup, IEnumerable targets, bool doExtensionConversion, bool ignoreNullRoot, ConversionToken? infoOwner = null, bool infoIsOfDocument = false) { var resultTargets = new List(); - if (TryGetRootTarget(root, ignoreNullRoot, out var rootTarget)) + if (TryGetRootTarget(root, ignoreNullRoot, infoOwner, infoIsOfDocument, out var rootTarget)) { resultTargets.Add(rootTarget.Value); } @@ -19,11 +28,14 @@ async Task VerifyInner(object? root, Func? cleanup, IEnumera cleanup = cleanup.Then(extraCleanup); resultTargets.AddRange(extraTargets); cleanup = RemoveExcludedTargets(resultTargets, cleanup, out var removedTargets); - if (removedTargets && - resultTargets.Count == 0) + // Removed here, or left out by a converter that asked what was excluded and so never + // produced them. Either way the exclusions account for there being nothing to verify + if (resultTargets.Count == 0 && + (removedTargets || + (conversionRan && VerifierSettings.AnyExcludedTargets(settings.Context)))) { await cleanup(); - throw new("All targets have been excluded by ExcludeTargets. A verification requires at least one target."); + throw new("All targets have been excluded by ExcludeTargets or ExcludeDerivedTargets. A verification requires at least one target."); } // Before this verification can queue, settle or retire anything of its own, so what a run @@ -328,7 +340,7 @@ Func RemoveExcludedTargets(List targets, Func cleanup, out b for (var index = targets.Count - 1; index >= 0; index--) { var target = targets[index]; - if (!VerifierSettings.IsExcluded(settings.Context, target.Extension)) + if (!IsExcluded(target)) { continue; } @@ -346,6 +358,19 @@ Func RemoveExcludedTargets(List targets, Func cleanup, out b return cleanup; } + // ExcludeDerivedTargets is asked only of what a converter derived from a document. The + // document, a target passed in directly, and the info this builds itself answer to + // ExcludeTargets alone + bool IsExcluded(in Target target) + { + if (target.IsDerived) + { + return VerifierSettings.IsDerivedExcluded(settings.Context, target.Extension); + } + + return VerifierSettings.IsExcluded(settings.Context, target.Extension); + } + async Task<(List extra, Func cleanup)> GetTargets(IEnumerable targets, bool doExtensionConversion) { var cleanup = () => Task.CompletedTask; @@ -368,7 +393,9 @@ Func RemoveExcludedTargets(List targets, Func cleanup, out b foreach (var target in list) { - if (!target.PerformConversion || + // A source is the document a typed converter gave, and is not converted again + if (target.IsSource || + !target.PerformConversion || !VerifierSettings.HasStreamConverter(target.Extension)) { Scrub(target); @@ -376,22 +403,30 @@ Func RemoveExcludedTargets(List targets, Func cleanup, out b continue; } - var (info, converted, itemCleanup) = await DoExtensionConversion(target, null); - cleanup = cleanup.Then(itemCleanup); - if (info != null) + var converted = await DoExtensionConversion(target, null); + cleanup = cleanup.Then(converted.Cleanup); + if (converted.Info != null) { - Target infoTarget = new( + var infoTarget = new Target( settings.TxtOrJson, JsonFormatter.AsJson( settings, counter, - info)); + converted.Info), + converted.InfoName) + { + // Of the source its conversion named, or of the one the converted target + // was itself derived from + Conversion = converted.InfoOwner ?? target.Conversion, + // As the info of a converted stream is: see TryGetRootTarget + DontInline = converted.InfoIsOfDocument + }; Scrub(infoTarget); result.Add(infoTarget); } // converted targets are scrubbed within DoExtensionConversion - result.AddRange(converted); + result.AddRange(converted.Targets); } return (result, cleanup); @@ -406,11 +441,40 @@ void Scrub(in Target target) } } - bool TryGetRootTarget(object? root,bool ignoreNullRoot, [NotNullWhen(true)] out Target? target) + bool TryGetRootTarget(object? root, bool ignoreNullRoot, ConversionToken? infoOwner, bool infoIsOfDocument, [NotNullWhen(true)] out Target? target) + { + if (!TryGetRootTarget(root, ignoreNullRoot, out target, out var hasAppends)) + { + return false; + } + + // What was appended is the test's, not the converter's, so a file holding any is not one + // the document alone accounts for + if (hasAppends) + { + return true; + } + + if (infoOwner is not null || + infoIsOfDocument) + { + target = target.Value with + { + Conversion = infoOwner, + // A file of the document, reviewed and accepted with the rest of them. Inlined by + // the global switch it would be the one part of the document that was not a file + DontInline = true + }; + } + + return true; + } + + bool TryGetRootTarget(object? root, bool ignoreNullRoot, [NotNullWhen(true)] out Target? target, out bool hasAppends) { var appends = VerifierSettings.GetJsonAppenders(settings); - var hasAppends = appends.Count > 0; + hasAppends = appends.Count > 0; if (ignoreNullRoot && root == null && !hasAppends) { diff --git a/src/Verify/Verifier/InnerVerifier_Object.cs b/src/Verify/Verifier/InnerVerifier_Object.cs index 7866b9e766..c3f51124c1 100644 --- a/src/Verify/Verifier/InnerVerifier_Object.cs +++ b/src/Verify/Verifier/InnerVerifier_Object.cs @@ -58,7 +58,9 @@ public async Task Verify(object? target) if (VerifierSettings.TryGetTypedConverter(target, settings, out var converter)) { var result = await converter.Conversion(target, settings.Context); - return await VerifyInner(result.Info, result.Cleanup, result.Targets, true, true); + conversionRan = true; + var targets = Adopt(result, null, null, false, out var token); + return await VerifyInner(result.Info, result.Cleanup, targets, true, true, token, result.IsDerivation); } return await VerifyInner(target, null, emptyTargets, true, false); diff --git a/src/Verify/Verifier/InnerVerifier_Stream.cs b/src/Verify/Verifier/InnerVerifier_Stream.cs index 14e7cea19f..6b024b55f8 100644 --- a/src/Verify/Verifier/InnerVerifier_Stream.cs +++ b/src/Verify/Verifier/InnerVerifier_Stream.cs @@ -92,9 +92,9 @@ public async Task VerifyStream(Stream? stream, string extension, o if (VerifierSettings.HasStreamConverter(extension)) { var initial = await GetTarget(stream, extension); - var (newInfo, converted, cleanup) = await DoExtensionConversion(initial, info); + var converted = await DoExtensionConversion(initial, info); - return await VerifyInner(newInfo, cleanup, converted, false, true); + return await VerifyInner(converted.Info, converted.Cleanup, converted.Targets, false, true, converted.InfoOwner, converted.InfoIsOfDocument); } var target = await GetTarget(stream, extension); @@ -124,7 +124,34 @@ static async Task GetTarget(Stream stream, string extension) return new(extension, stream); } - async Task<(object? info, List targets, Func cleanup)> DoExtensionConversion(Target initial, object? info) + /// + /// What converting one target, and whatever its conversion produced that could itself be + /// converted, came to. + /// + /// The infos of the conversions, with the one passed in first. + /// The targets left once nothing more could be converted. + /// Disposes what the conversions own. + /// + /// The conversion belongs to, where it is nothing but what converters + /// said of a source the first of them named. Its info file is then derived from that source. + /// + /// + /// The name of the info file, where the first conversion names its targets relative to the + /// target it converted: that target's name. + /// + /// + /// is nothing but what converters said of a document the first of + /// them told apart from what it derived, whether or not that document is itself a target. It + /// is then a file of the document, and stays one: see . + /// + readonly record struct Converted(object? Info, List Targets, Func Cleanup, ConversionToken? InfoOwner, string? InfoName, bool InfoIsOfDocument); + + // A converter ran for this verification. A converter that is told what is excluded leaves + // those targets out itself, so having none left can be the exclusions' doing with nothing + // having been removed here: see VerifyInner + bool conversionRan; + + async Task DoExtensionConversion(Target initial, object? info) { var cleanup = () => Task.CompletedTask; // the source stream of a stream target is owned here, so dispose it once consumed @@ -140,6 +167,10 @@ static async Task GetTarget(Stream stream, string extension) } var targets = new List(); + ConversionToken? infoOwner = null; + string? infoName = null; + var infoIsOfDocument = false; + var isInitial = true; var queue = new Queue(); queue.Enqueue(initial); @@ -148,7 +179,12 @@ static async Task GetTarget(Stream stream, string extension) { var target = queue.Dequeue(); - if (!VerifierSettings.TryGetStreamConverter(target.Extension, out var conversion)) + // A source is the document as its conversion gave it, so it is not converted again + // whatever its extension: a doc given back as a docx is not then run through the docx + // converter. PerformConversion is how any other target asks for the same + if (target.IsSource || + !target.PerformConversion || + !VerifierSettings.TryGetStreamConverter(target.Extension, out var conversion)) { // terminal target: scrub text before it is finalized Scrub(target); @@ -184,7 +220,24 @@ static async Task GetTarget(Stream stream, string extension) infos.Add(result.Info); } - var resultTargets = result.Targets.ToList(); + conversionRan = true; + var resultTargets = Adopt(result, target.Name, target.Conversion, target.IsDerived, out var token); + if (isInitial) + { + isInitial = false; + // With an info passed in, the file holds that as well, and is not the source's + if (info is null && + result.Source is not null) + { + infoOwner = token; + } + + if (result.IsDerivation) + { + infoName = target.Name; + infoIsOfDocument = info is null; + } + } foreach (var resultTarget in resultTargets) { @@ -207,6 +260,85 @@ static async Task GetTarget(Stream stream, string extension) > 1 => infos, _ => null }; - return (newInfo, targets, cleanup); + return new(newInfo, targets, cleanup, infoOwner, infoName, infoIsOfDocument); + } + + /// + /// Takes what a conversion returned into the verification. The one place a + /// is made, and where the targets of a conversion that told + /// its source from what it derived are named and tied to that source. + /// + /// What the conversion returned. + /// The name of the target that was converted, or null. + /// The conversion that target came out of, or null. + /// Whether that target was itself derived from a document. + /// + /// The conversion the targets belong to: a new one where names a + /// source, and otherwise , since what was computed from a derived + /// target was derived from the same source it was. + /// + static List Adopt(in ConversionResult result, string? name, ConversionToken? parent, bool derived, out ConversionToken? token) + { + if (!result.IsDerivation) + { + // Named by the converter, which was passed the name to do it with + token = parent; + if (!derived) + { + return result.Targets.ToList(); + } + + // Computed from a derived target, so derived from whatever that was. Its source where + // it has one, and derived all the same where the source was left out + return result.Targets + .Select(_ => _ with { Conversion = parent, IsDerived = true }) + .ToList(); + } + + var hasSource = result.Source is not null; + if (hasSource) + { + token = new(parent); + } + else + { + token = parent; + } + + var targets = new List(); + foreach (var target in result.Targets) + { + // The source is the first of the targets where there is one + var isSource = hasSource && targets.Count == 0; + targets.Add( + target with + { + Name = RelativeName(name, target.Name), + Conversion = token, + IsSource = isSource, + IsDerived = !isSource + }); + } + + return targets; + } + + /// + /// A target named page_0001, from the conversion of a target named Attachment1, + /// is Attachment1.page_0001. Either on its own where the other is missing. + /// + static string? RelativeName(string? converted, string? name) + { + if (converted is null) + { + return name; + } + + if (name is null) + { + return converted; + } + + return $"{converted}.{name}"; } } \ No newline at end of file diff --git a/src/Verify/Verifier/RaisedDeletes.cs b/src/Verify/Verifier/RaisedDeletes.cs index 8a2b74ae6c..dd9721f682 100644 --- a/src/Verify/Verifier/RaisedDeletes.cs +++ b/src/Verify/Verifier/RaisedDeletes.cs @@ -27,8 +27,19 @@ static class RaisedDeletes /// /// Swapped in tests. What reaches the tray or the viewer is otherwise only observable from them. + /// The second argument is the received file of the source the file was derived from, or null. /// - internal static Func AddDelete = DiffRunner.AddDeleteAsync; + internal static Func AddDelete = DefaultAddDelete; + + static Task DefaultAddDelete(string file, string? source) + { + if (source is null) + { + return DiffRunner.AddDeleteAsync(file); + } + + return DiffRunner.AddDerivedDeleteAsync(file, source); + } /// internal static Action SettleDelete = DiffRunner.SettleDelete; @@ -51,7 +62,13 @@ static class RaisedDeletes /// Raises a delete for a verified file no target produced, recording it first so it is never /// pending without a record. /// - public static Task Raise(string file) + /// The verified file. + /// + /// The received file of the pending source the file is taken to have been derived from, a page + /// a document no longer has, or null for a file that stands alone. Withdrawing the delete + /// later goes by the file alone. + /// + public static Task Raise(string file, string? source) { // Nothing is raised where DiffEngine is switched off, so there is nothing to withdraw later if (!DiffRunner.Disabled) @@ -59,7 +76,7 @@ public static Task Raise(string file) Record(file); } - return AddDelete(file); + return AddDelete(file, source); } /// diff --git a/src/Verify/Verifier/VerifyEngine.cs b/src/Verify/Verifier/VerifyEngine.cs index da460c8138..b4198db6e2 100644 --- a/src/Verify/Verifier/VerifyEngine.cs +++ b/src/Verify/Verifier/VerifyEngine.cs @@ -69,60 +69,103 @@ public async Task HandleResults(List targetList) var file = getFileNames(target); SeedMigrated(file); + Track(target, file); var result = await GetResult(settings, file, target, false, false); HandleCompareResult(result, file); return; } + var planned = Plan(targetList); + var results = new EqualityResult[planned.Count]; var textHasFailed = false; var bypassComparers = false; - // The inlined target is compared outside Inner, so it has to feed the same cascade by - // hand. Without this a first target that differed told the targets after it nothing, and - // the derived ones kept trusting comparers that exist to tolerate differences the source - // target had just failed on - which is what BypassComparersForSubsequentOnDifference is - // for, and it stopped working as soon as that target was the inlined one - async Task CompareInline(Target target) + // A target that differed tells the ones compared after it. The inlined target is compared + // by its own engine, so it has to feed the same cascade by hand. Without that a first + // target that differed told the targets after it nothing, and the derived ones kept + // trusting comparers that exist to tolerate differences the source target had just failed + // on - which is what BypassComparersForSubsequentOnDifference is for, and it stopped + // working as soon as that target was the inlined one + void NoteDifference(in Target target, bool isText) { - await inlineEngine!.Compare(target); - if (inlineEngine.Equality == Equality.Equal) + if (isText) { - return; + textHasFailed = true; } - // Always text: Compare refuses a stream outright - textHasFailed = true; if (target.BypassComparersForSubsequentOnDifference) { bypassComparers = true; } + + if (target is { IsSource: true, Conversion: { } conversion }) + { + differed.Add(conversion); + } } - async Task Inner(FilePair file, Target target, bool isFirst) + foreach (var index in CompareOrder(planned)) { + var (target, planFile, isFirst) = planned[index]; + if (planFile is not { } file) + { + await inlineEngine!.Compare(target); + if (inlineEngine.Equality != Equality.Equal) + { + // Always text: Compare refuses a stream outright + NoteDifference(target, true); + } + + continue; + } + if (isFirst) { SeedMigrated(file); } - var result = await GetResult(settings, file, target, textHasFailed, bypassComparers); + Track(target, file); + // A text target that failed has always made the binary targets after it skip their + // comparers: with nothing else to go on, it is the sign that what they were made from + // has changed. A target whose source was compared has something better to go on, and + // a source that passed says its pages are the pages they were. Without this, an info + // file that changed shape had every page of an unchanged document compared exactly. + var textFailed = textHasFailed && !SourceCompared(target); + var result = await GetResult(settings, file, target, textFailed, bypassComparers || SourceDiffered(target)); if (result.Equality != Equality.Equal) { - if (file.IsText) - { - textHasFailed = true; - } - - if (target.BypassComparersForSubsequentOnDifference) - { - bypassComparers = true; - } + NoteDifference(target, file.IsText); } - HandleCompareResult(result, file); + results[index] = result; } + // In the order planned rather than the order compared, so the callbacks and the exception + // message list the files as they always have + for (var index = 0; index < planned.Count; index++) + { + if (planned[index].File is { } file) + { + HandleCompareResult(results[index], file); + } + } + } + + /// + /// A target and the file it is compared as. No file for the target that is inlined. + /// + readonly record struct Planned(Target Target, FilePair? File, bool IsFirst); + + /// + /// Every target with the file it is compared as. Decided for all of them before any is + /// compared, so that the order they are compared in is free to differ from the order that + /// names them. + /// + List Plan(List targetList) + { + var planned = new List(targetList.Count); + // Grouped over the full list, including the inlined target, so the file names of the // remaining targets are the same whether or not inline is on. That leaves a deliberate // gap where the inlined target's #00 would have been. @@ -140,16 +183,159 @@ async Task Inner(FilePair file, Target target, bool isFirst) var (target, position) = targets[index]; if (position == 0 && inlineEngine is not null) { - await CompareInline(target); + planned.Add(new(target, null, true)); continue; } var file = indexNames ? getIndexedFileNames(target, index.ToString("D2")) : getFileNames(target); - await Inner(file, target, position == 0); + planned.Add(new(target, file, position == 0)); } } + + return planned; + } + + /// + /// The order the planned targets are compared in: the inlined one, then the sources, then the + /// rest, each as planned. + /// + /// + /// A source is compared ahead of what was derived from it, because how the derived targets + /// are compared depends on what its comparison found (). Planned + /// order alone does not give that: an info file is the first target and its document the + /// second. Outermost first among the sources, since a source rendered from another answers to + /// that one the same way. + /// + static IEnumerable CompareOrder(List planned) + { + var sources = new List(); + var rest = new List(); + for (var index = 0; index < planned.Count; index++) + { + var item = planned[index]; + if (item.File is null) + { + yield return index; + } + else if (item.Target.IsSource) + { + sources.Add(index); + } + else + { + rest.Add(index); + } + } + + // A stable sort, so sources as deep as each other stay as planned + foreach (var index in sources.OrderBy(_ => Depth(planned[_].Target.Conversion))) + { + yield return index; + } + + foreach (var index in rest) + { + yield return index; + } + } + + static int Depth(ConversionToken? conversion) + { + var depth = 0; + while (conversion is not null) + { + depth++; + conversion = conversion.Parent; + } + + return depth; + } + + // The conversion each compared file came out of, by received path, and the file the source of + // each conversion was compared as. Together, what ties a file to the document it was derived + // from once the targets themselves have gone + Dictionary derivations = []; + Dictionary sources = []; + + // The conversions whose source was new or not equal + HashSet differed = []; + + readonly record struct Derivation(ConversionToken Conversion, bool IsSource); + + readonly record struct SourceFile(FilePair File, bool IsNamed); + + void Track(in Target target, in FilePair file) + { + if (target.Conversion is not { } conversion) + { + return; + } + + derivations[file.ReceivedPath] = new(conversion, target.IsSource); + if (target.IsSource) + { + sources[conversion] = new(file, target.Name is not null); + } + } + + /// + /// Whether a source this target was derived from differed, in which case the target skips its + /// registered comparer and is compared exactly. A comparer exists to tolerate differences, and + /// a document that has changed is the one case where a page of it should not be given the + /// benefit of the doubt. + /// + /// + /// Only the sources above the target. One conversion's document differing says nothing of the + /// pages of another's, which is what the flag this replaces, + /// , could not express. + /// + /// + /// Whether a source this target was derived from is itself a target of the verification, and + /// so has been compared: sources are compared ahead of everything derived from them. + /// + bool SourceCompared(in Target target) + { + if (target.IsSource) + { + return false; + } + + var conversion = target.Conversion; + while (conversion is not null) + { + if (sources.ContainsKey(conversion)) + { + return true; + } + + conversion = conversion.Parent; + } + + return false; + } + + bool SourceDiffered(in Target target) + { + var conversion = target.Conversion; + // A source answers to the sources above it, not to itself + if (target.IsSource) + { + conversion = conversion?.Parent; + } + + while (conversion is not null) + { + if (differed.Contains(conversion)) + { + return true; + } + + conversion = conversion.Parent; + } + + return false; } /// @@ -228,11 +414,22 @@ public async Task ThrowIfRequired() return; } - var allDeletesVerified = await ProcessDeletes(); + bool allDeletesVerified; + bool allNewVerified; + bool allNotEqualsVerified; + try + { + allDeletesVerified = await ProcessDeletes(); - var allNewVerified = await ProcessNew(); + allNewVerified = await ProcessNew(); - var allNotEqualsVerified = await ProcessNotEquals(); + allNotEqualsVerified = await ProcessNotEquals(); + } + finally + { + // Whatever the three left pending, including when a callback of one threw part way + await Report(); + } var (allInlineVerified, inlineSection) = await ProcessInline(inlineFailed); @@ -331,7 +528,7 @@ async Task ProcessDeletes(string file) return true; } - await RaisedDeletes.Raise(file); + pendingDeletes.Add(file); return false; } @@ -366,7 +563,7 @@ async Task ProcessNotEquals() await VerifierSettings.RunAddTestAttachment(notEqual.File.ReceivedPath); var autoVerify = IsAutoVerify(notEqual.File.VerifiedPath); await settings.RunOnVerifyMismatch(notEqual.File, notEqual.Message, autoVerify); - if (!await RunDiffAutoCheck(notEqual.File, autoVerify)) + if (!RunDiffAutoCheck(notEqual.File, autoVerify)) { verified = false; } @@ -388,37 +585,252 @@ void ProcessEquals() } } - // ReSharper disable once UnusedParameter.Local - // ReSharper disable once MemberCanBeMadeStatic.Local - async Task RunDiffAutoCheck(FilePair file, bool autoVerify) + bool RunDiffAutoCheck(FilePair file, bool autoVerify) { if (autoVerify) { autoVerified.Add(file); + AcceptChanges(file); + return true; } - if (autoVerify) + // The received file is being left on disk. It is reported once every pending file is + // known: see Report + pendingMoves.Add(file); + + return false; + } + + // The received files left on disk, and the verified files no target produced, that were not + // auto verified: what Report tells DiffEngine of + List pendingMoves = []; + List pendingDeletes = []; + + /// + /// Swapped in tests. What reaches a diff tool is otherwise only observable from one. + /// The second argument is the received file of the source the pair was derived from, or null. + /// + internal static Func LaunchDiff = DefaultLaunchDiff; + + static Task DefaultLaunchDiff(FilePair file, string? source) + { + var encoding = VerifierSettings.Encoding; + if (source is null) { - AcceptChanges(file); - return autoVerify; + if (file.IsText) + { + return DiffRunner.LaunchForTextAsync(file.ReceivedPath, file.VerifiedPath, encoding); + } + + return DiffRunner.LaunchAsync(file.ReceivedPath, file.VerifiedPath, encoding); + } + + if (file.IsText) + { + return DiffRunner.LaunchDerivedForTextAsync(file.ReceivedPath, file.VerifiedPath, source, encoding); + } + + return DiffRunner.LaunchDerivedAsync(file.ReceivedPath, file.VerifiedPath, source, encoding); + } + + /// + /// Tells DiffEngine, and the received maps, of everything this verification left pending. + /// + /// + /// Once, after the last of it is known, rather than file by file as each is processed. A file + /// derived from a document says so only while the document is itself pending, and DiffEngine + /// has to hear of the document before it hears of anything derived from it, so that a tool + /// drawing the document can show its pages beneath it rather than each in a window of its own. + /// Neither can be known while the files are still being gone through: the document is the + /// second target more often than the first. + /// + /// Deletes that stand alone stay ahead of the moves, as they always were. A delete derived + /// from a document follows the launch for that document. + /// + /// + async Task Report() + { + var pending = new HashSet(pendingMoves.Select(_ => _.ReceivedPath)); + + var moves = new List<(FilePair file, string? source)>(pendingMoves.Count); + foreach (var file in pendingMoves) + { + moves.Add((file, SourceOf(file, pending))); } + var deletes = new List<(string file, string? source)>(pendingDeletes.Count); + foreach (var file in pendingDeletes) + { + deletes.Add((file, SourceOfStale(file, pending))); + } + + foreach (var (file, source) in deletes) + { + if (source is null) + { + await RaisedDeletes.Raise(file, null); + } + } + + foreach (var (file, source) in moves) + { + if (source is null) + { + await ReportMove(file, null); + } + } + + foreach (var (file, source) in moves) + { + if (source is not null) + { + await ReportMove(file, source); + } + } + + foreach (var (file, source) in deletes) + { + if (source is not null) + { + await RaisedDeletes.Raise(file, ForDiffEngine(source)); + } + } + } + + async Task ReportMove(FilePair file, string? source) + { // The received file is being left on disk, so record the verified file it belongs to. - ReceivedMap.Write(file); + // With its source whether or not a diff tool is told: the map is for tooling that finds + // the files on disk, where the source is as much there as the file is + ReceivedMap.Write(file, source); if (diffEnabled) { - if (file.IsText) + await LaunchDiff(file, source); + } + } + + // Nothing was launched for a source while diff is off, so there is nothing for DiffEngine to + // show a derived file beneath + string? ForDiffEngine(string source) + { + if (diffEnabled) + { + return source; + } + + return null; + } + + /// + /// The received file of the source a pending file was derived from, or null for a file that + /// stands alone: one no conversion derived, the source itself, or a file whose source passed + /// or was auto verified and so has no received file to name. + /// + string? SourceOf(in FilePair file, HashSet pending) + { + if (!derivations.TryGetValue(file.ReceivedPath, out var derivation)) + { + return null; + } + + // A source is derived from the sources above it, not from itself + if (derivation.IsSource) + { + return OutermostPending(derivation.Conversion.Parent, pending); + } + + return OutermostPending(derivation.Conversion, pending); + } + + /// + /// The outermost source, of this conversion and those it came out of, that is pending. + /// + /// + /// One level is all a diff tool is told: a page of a pdf that was rendered from a docx is a + /// file of the docx, as the pdf is. The outermost because that is the document the test + /// verified, and so the one a reviewer is looking at. + /// + string? OutermostPending(ConversionToken? conversion, HashSet pending) + { + string? source = null; + while (conversion is not null) + { + if (sources.TryGetValue(conversion, out var candidate) && + pending.Contains(candidate.File.ReceivedPath)) { - await DiffRunner.LaunchForTextAsync(file.ReceivedPath, file.VerifiedPath, VerifierSettings.Encoding); + source = candidate.File.ReceivedPath; } - else + + conversion = conversion.Parent; + } + + return source; + } + + /// + /// The source a verified file that no target produced is taken to have been derived from: a + /// page a document no longer has. + /// + /// + /// There is no target to say, so it goes by the name: the file of a pending source that was + /// named when its own name carries on from that one, and otherwise the one pending source + /// that was not named, where there is exactly one. Anything else stands alone, which costs a + /// reviewer one more accept and never a wrong one. + /// + string? SourceOfStale(string verifiedFile, HashSet pending) + { + ConversionToken? named = null; + var namedLength = 0; + var unnamed = new HashSet(); + foreach (var (conversion, source) in sources) + { + if (!pending.Contains(source.File.ReceivedPath)) + { + continue; + } + + if (!source.IsNamed) + { + // Sources rendered from one another are one document to a reviewer + unnamed.Add(OutermostPending(conversion, pending)!); + continue; + } + + var stem = Stem(source.File); + if (stem.Length > namedLength && + verifiedFile.StartsWith($"{stem}.", StringComparison.OrdinalIgnoreCase)) { - await DiffRunner.LaunchAsync(file.ReceivedPath, file.VerifiedPath, VerifierSettings.Encoding); + named = conversion; + namedLength = stem.Length; } } - return autoVerify; + if (named is not null) + { + return OutermostPending(named, pending); + } + + if (unnamed.Count == 1) + { + return unnamed.Single(); + } + + return null; + } + + // A verified path less what follows the name: ".verified.pdf", or ".pdf" in a directory of + // verified files + static string Stem(in FilePair file) + { + var path = file.VerifiedPath; + var suffix = $".verified.{file.Extension}"; + if (path.EndsWith(suffix, StringComparison.OrdinalIgnoreCase)) + { + return path[..^suffix.Length]; + } + + return path[..^(file.Extension.Length + 1)]; } async Task ProcessNew() @@ -429,7 +841,7 @@ async Task ProcessNew() await VerifierSettings.RunAddTestAttachment(file.File.ReceivedPath); var autoVerify = IsAutoVerify(file.File.VerifiedPath); await settings.RunOnFirstVerify(file, autoVerify); - if (!await RunDiffAutoCheck(file.File, autoVerify)) + if (!RunDiffAutoCheck(file.File, autoVerify)) { verified = false; }