Deduplicate path separator duplicates by philderbeast · Pull Request #10646 · haskell/cabal

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.

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

Kleidukos · 2025-01-14T16:07:42Z

@Mergifyio backport 3.14

mergify · 2025-01-14T16:08:07Z

backport 3.14

✅ Backports have been created

Details

#10751 Backport #10646: Deduplicate path separator duplicates has been created for branch 3.14 but encountered conflicts

Kleidukos · 2025-01-14T17:59:18Z

@Mergifyio backport 3.14

mergify · 2025-01-14T17:59:36Z

backport 3.14

✅ Backports have been created

Details

#10751 Backport #10646: Deduplicate path separator duplicates has been created for branch 3.14

Kleidukos · 2025-01-14T18:00:44Z

bloody hell.

Kleidukos · 2025-01-14T18:00:46Z

@Mergifyio backport 3.14

mergify · 2025-01-14T18:01:02Z

backport 3.14

✅ Backports have been created

Details

#10751 Backport #10646: Deduplicate path separator duplicates has been created for branch 3.14

Kleidukos · 2025-01-14T18:01:51Z

curse you, mergify!

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 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

mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Jan 5, 2025

mergify bot merged commit 2040d21 into haskell:master Jan 5, 2025
53 checks passed

This was referenced Jan 7, 2025

Add source file to project parse errors and warnings #10644

Merged

Implement assert matches with assert on #10735

Merged

mergify bot mentioned this pull request Jan 14, 2025

Backport #10646: Deduplicate path separator duplicates #10751

Closed

6 tasks

andreasabel mentioned this pull request Apr 15, 2025

Test DedupUsingConfigFromComplex flakey? #10927

Closed

philderbeast mentioned this pull request Aug 26, 2025

Fix haddock typo: coversion #11190

Merged

2 tasks

Conversation

philderbeast commented Dec 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

geekosaur left a comment

Choose a reason for hiding this comment

Uh oh!

philderbeast commented Dec 17, 2024

Uh oh!

geekosaur commented Dec 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

philderbeast commented Dec 18, 2024

Uh oh!

philderbeast commented Dec 31, 2024

Uh oh!

philderbeast commented Jan 1, 2025

Uh oh!

Uh oh!

Kleidukos commented Jan 14, 2025

Uh oh!

mergify bot commented Jan 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Backports have been created

Uh oh!

Kleidukos commented Jan 14, 2025

Uh oh!

mergify bot commented Jan 14, 2025

✅ Backports have been created

Uh oh!

Kleidukos commented Jan 14, 2025

Uh oh!

Kleidukos commented Jan 14, 2025

Uh oh!

mergify bot commented Jan 14, 2025

✅ Backports have been created

Uh oh!

Kleidukos commented Jan 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

philderbeast commented Dec 17, 2024 •

edited

Loading

geekosaur commented Dec 17, 2024 •

edited

Loading

mergify bot commented Jan 14, 2025 •

edited

Loading