Deduplicate path separator duplicates #10646

philderbeast · 2024-12-17T16:46:51Z

Fixes #10645, a small Windows gotcha missed with #10546.

I really had to get into the weeds with this one. The assertOutputContains function was modifying the output (a test actual value) before comparision with the unmodified actual value. This change was not visible. I've added an assertOn function (that assertOutputContains calls) that takes a NeedleHaystack configuration for how the search, expectation and display are made. I had wanted to add a pilcrow ¶ for line endings but the terminal output is restricted to ASCII so I've marked line beginnings with ^ and line endings with $. The needle (the expected output fragment) is shown annotated this way as can the haystack (the output). The haystack is not shown annotated with line delimiters but can be.

The concatOutput function, I've renamed to lineBreaksToSpaces. There's more detail on the cabal-testsuite changes in the changelog entry. One benefit of using an .md extension for changelog.d/pr-10646.md is that typos are caught by CI.

Patches conform to the coding conventions.
Any changes that could be relevant to users have been recorded in the changelog.
- Is the change significant? If so, remember to add significance: significant in the changelog file.
The documentation has been updated, if necessary.
Manual QA notes have been included.
Tests have been added. Yes, a doctest.

geekosaur

I'm confused: is the .md file a GitHub artifact, or did you really add two changelog.d entries?

philderbeast · 2024-12-17T17:15:38Z

I'm confused: is the .md file a GitHub artifact, or did you really add two changelog.d entries?

Thanks for catching that mistake of mine @geekosaur. I let the .md slip in. I rename locally to check the markdown preview. I've deleted that extra file now.

Can we use a .md extension for changelog.d entries?

geekosaur · 2024-12-17T17:19:58Z

changelog-d doesn't care; the only special name is config, where its own configuration is stored, and you can't use subdirectories. It might even be better to use the extension, since that way GitHub will also render it (although it will probably be confused by the YAML frontmatter).

philderbeast · 2024-12-18T11:08:11Z

Reverting to draft while I settle some Windows versus POSIX file path issues.

cabal-testsuite/src/Test/Cabal/NeedleHaystack.hs

cabal-testsuite/src/Test/Cabal/Prelude.hs

changelog.d/pr-10646.md

Mikolaj

That was a tricky one, but the tests make it easier for the reviewer to trust the solution without spending hours to get a deep understanding. The PR will be a good test of our tools and the process. I think it would be great to also include this great write-up from the changelog in a Note or maybe in a test linked to by name from the code?

philderbeast · 2024-12-31T17:29:05Z

@Mikolaj I've added a note [Multiline Needles] and referenced this from assertOn and assertOutputContains with pretty much the content from the changelog but converted to haddock format and without the doctest output that can be found elsewhere in cabal-testsuite/src/Test/Cabal/NeedleHaystack.hs.

philderbeast · 2024-12-31T17:31:58Z

That was a tricky one

Yes it was and I'll have to squash those ~70 commits before applying a merge label.

philderbeast · 2025-01-01T13:40:02Z

Label merge+no rebase is necessary when the pull request is from an organisation.

- Manually replace path separators before anything else - Use pathSeparator instead of literal char - Use isPathSeparator predicate

- Add else.project test - Use normalizeWindowsOutput - Add a changelog entry - Update expectation - Use concatOutput on needle - Include output - Align lines - Show modified output - Apply concatOutput to the needle - Show start and end of lines with ASCII ^ and $h - Can't print pilcrow so use grep char for marking end of line - Marking the start of line distinguishes "expected" intro from its content too, same for "output" - Use \n in multiline string expectation - Add NeedleHaystack - Add expectNeedleInHaystack field to NeedleHaystack - Remove 3 assert*Contains functions - Add TxContains record - Apply the txBwd transformations before display - Add displayHaystack field - Switch to using <EOL> as the marker - Sort language pragmas - Use ++ rather than cons with reversals - Rerun ParseErrorProvenance test - Add doctests for single line strings - Read exected multiline string from file - Use lineBreaksToSpaces - Add module Test.Cabal.NeedleHaystack - Redo ConditionalAndImport with multiline expectations - Add test of string expectation start and end marking - Rename encodeLf and decodeLfMarkLines - Rename original concatOutput to lineBreaksToSpaces - Add assertOutputContainsWrapped - Use multiline and wrapped assertions - DedupUsingConfigFromComplex multiline assertion - Remove redundant tests that fail on Windows - Use normalizeWindowsOutput in ConditionalAndImport - Forward conversion applied twice by mistake - Easier diff when assertOn follows assertOutputContains - Add readVerbatimFile - Have readVerbatimFile read contents strictly - Add normalizePathSeparator - Don't modify path separator for URIs - Don't normalize path with anything URI-like - Normalize expected output - Rename to normalizePathSeparators - Add an explicit export list to NeedleHaystack - Drop unlines . lines added trailing newline - Show example of normalizePathSeparators - Use local unsnoc definition to avoid CPP - Define local unlines - Satisfy fix-whitespace - Don't use <EOL> - Rename to delimitLines - Rename the changelog with *.md extension - Add a section on cabal-testsuite changes - Rename the function to readFileVerbatim - Add to contributing and cabal-testsuite's readme - Use setup for the noun - Typo s/displaying/display - Typo "can easier" - Use unsnoc from Cabal-syntax Utils.Generic - Add a note [Multiline Needles] - Remove doctests available elsewhere - Substitute encodeLf for concatOutput for assertOutputMatches

- Add oops.expect.txt - Add cabal-missing-package.expect.txt - Add hops.expect.txt - Add DedupUsingConfigFromComplex/errors.expect.txt - Add using configuration from to errors.expect.txt

philderbeast changed the title ~~Configuration from message path separator duplicates~~ Deduplicate path separator duplicates Dec 17, 2024

philderbeast force-pushed the fix/path-sep-duplicates branch from 2d03f00 to 53fd7b0 Compare December 17, 2024 17:02

geekosaur approved these changes Dec 17, 2024

View reviewed changes

philderbeast force-pushed the fix/path-sep-duplicates branch from 53fd7b0 to ca61a07 Compare December 17, 2024 17:13

philderbeast marked this pull request as draft December 18, 2024 11:07

philderbeast force-pushed the fix/path-sep-duplicates branch 19 times, most recently from bfda13a to 14850de Compare December 24, 2024 02:53

philderbeast marked this pull request as ready for review December 24, 2024 03:25

philderbeast added attention: needs-review cabal-testsuite labels Dec 24, 2024

philderbeast requested a review from geekosaur December 24, 2024 03:26

geekosaur approved these changes Dec 24, 2024

View reviewed changes

philderbeast force-pushed the fix/path-sep-duplicates branch 2 times, most recently from 4a8c795 to ead2ec4 Compare December 30, 2024 18:00

Mikolaj approved these changes Dec 31, 2024

View reviewed changes

philderbeast force-pushed the fix/path-sep-duplicates branch from b638eac to 8cfe16c Compare December 31, 2024 17:26

philderbeast force-pushed the fix/path-sep-duplicates branch from 8cfe16c to c1122ef Compare December 31, 2024 17:30

philderbeast force-pushed the fix/path-sep-duplicates branch 4 times, most recently from 24af359 to ed55098 Compare January 1, 2025 13:27

philderbeast added merge+no rebase and removed attention: needs-review labels Jan 1, 2025

mergify bot added the ready and waiting Mergify is waiting out the cooldown period label Jan 1, 2025

Use OS-specific splitPath before comparing projects

d0bd687

- Manually replace path separators before anything else - Use pathSeparator instead of literal char - Use isPathSeparator predicate

philderbeast force-pushed the fix/path-sep-duplicates branch from ed55098 to 5d3c1a0 Compare January 3, 2025 10:53

philderbeast added 3 commits January 3, 2025 08:57

Fix up existing tests with multiline expectations

484ac9c

Add verbatim *.expect.txt expectations

32b820b

- Add oops.expect.txt - Add cabal-missing-package.expect.txt - Add hops.expect.txt - Add DedupUsingConfigFromComplex/errors.expect.txt - Add using configuration from to errors.expect.txt

philderbeast force-pushed the fix/path-sep-duplicates branch from 5539c23 to 32b820b Compare January 3, 2025 13:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deduplicate path separator duplicates #10646

Deduplicate path separator duplicates #10646

philderbeast commented Dec 17, 2024 •

edited

Loading

geekosaur left a comment

philderbeast commented Dec 17, 2024

geekosaur commented Dec 17, 2024 •

edited

Loading

philderbeast commented Dec 18, 2024

Mikolaj left a comment

philderbeast commented Dec 31, 2024

philderbeast commented Dec 31, 2024

philderbeast commented Jan 1, 2025

Deduplicate path separator duplicates #10646

Are you sure you want to change the base?

Deduplicate path separator duplicates #10646

Conversation

philderbeast commented Dec 17, 2024 • edited Loading

geekosaur left a comment

Choose a reason for hiding this comment

philderbeast commented Dec 17, 2024

geekosaur commented Dec 17, 2024 • edited Loading

philderbeast commented Dec 18, 2024

Mikolaj left a comment

Choose a reason for hiding this comment

philderbeast commented Dec 31, 2024

philderbeast commented Dec 31, 2024

philderbeast commented Jan 1, 2025

philderbeast commented Dec 17, 2024 •

edited

Loading

geekosaur commented Dec 17, 2024 •

edited

Loading