Skip to content

Commit

Permalink
Rename the changelog with *.md extension
Browse files Browse the repository at this point in the history
- Add a section on cabal-testsuite changes
- Rename the function to readFileVerbatim
  • Loading branch information
philderbeast committed Dec 24, 2024
1 parent 21dfa22 commit 14850de
Show file tree
Hide file tree
Showing 6 changed files with 216 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ main = cabalTest . withRepo "repo" . recordMode RecordMarked $ do
log "checking that imports work skipping into a subfolder and then back out again and again"
hopping <- cabal' "v2-build" [ "--project-file=hops-0.project" ]

readVerbatimFile "hops.expect.txt" >>=
readFileVerbatim "hops.expect.txt" >>=
flip (assertOn multilineNeedleHaystack) hopping . normalizePathSeparators

-- The project is named oops as it is like hops but has conflicting constraints.
Expand All @@ -129,7 +129,7 @@ main = cabalTest . withRepo "repo" . recordMode RecordMarked $ do
log "checking conflicting constraints skipping into a subfolder and then back out again and again"
oopsing <- fails $ cabal' "v2-build" [ "all", "--project-file=oops-0.project" ]

readVerbatimFile "oops.expect.txt"
readFileVerbatim "oops.expect.txt"
>>= flip (assertOn multilineNeedleHaystack) oopsing . normalizePathSeparators

-- The project is named yops as it is like hops but with y's for forks.
Expand Down Expand Up @@ -172,7 +172,7 @@ main = cabalTest . withRepo "repo" . recordMode RecordMarked $ do
log "checking that missing package message lists configuration provenance"
missing <- fails $ cabal' "v2-build" [ "--project-file=cabal-missing-package.project" ]

readVerbatimFile "cabal-missing-package.expect.txt"
readFileVerbatim "cabal-missing-package.expect.txt"
>>= flip (assertOn multilineNeedleHaystack) missing . normalizePathSeparators

return ()
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ main = cabalTest . recordMode RecordMarked $ do
log "check project configuration with URI imports is listed in full and"
log "check package directories and locations are reported in order"

readVerbatimFile "errors.expect.txt"
readFileVerbatim "errors.expect.txt"
>>= flip (assertOn multilineNeedleHaystack) out . normalizePathSeparators

return ()
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ main = cabalTest . recordMode RecordMarked $ do

outElse <- fails $ cabal' "v2-build" [ "all", "--dry-run", "--project-file=else.project" ]

msg <- readVerbatimFile "msg.expect.txt"
msg <- readFileVerbatim "msg.expect.txt"
let msgSingle = lineBreaksToSpaces msg

log "Multiline string marking:"
Expand Down
4 changes: 2 additions & 2 deletions cabal-testsuite/src/Test/Cabal/Prelude.hs
Original file line number Diff line number Diff line change
Expand Up @@ -1292,8 +1292,8 @@ findDependencyInStore pkgName = do
-- > "\\\\?\\C:\\Users\\<username>\\AppData\\Local\\Temp\\cabal-testsuite-8376\\errors.expect.txt":
-- > permission denied (The process cannot access the file because it is being
-- > used by another process.)
readVerbatimFile :: FilePath -> TestM String
readVerbatimFile filename = do
readFileVerbatim :: FilePath -> TestM String
readFileVerbatim filename = do
testDir <- testCurrentDir <$> getTestEnv
s <- liftIO . readFile $ testDir </> filename
length s `seq` return s
34 changes: 0 additions & 34 deletions changelog.d/pr-10646

This file was deleted.

209 changes: 209 additions & 0 deletions changelog.d/pr-10646.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,209 @@
---
synopsis: Configuration messages without duplicates
packages: [cabal-install-solver]
prs: 10646
issues: 10645
---

The "using configuration from" message no longer has duplicates on Windows when
a `cabal.project` uses forward slashes for its imports but the message reports
the same import again with backslashes.

```diff
$ cat cabal.project
import: dir-a/b.config

$ cabal build all --dry-run
...
When using configuration from:
- - dir-a/b.config
- dir-a\b.config
- cabal.project
```

## Changed `Ord ProjectConfigPath` Instance

For comparison purposes, path separators are normalized to the `buildOS`
platform's path separator.

```haskell
-- >>> let abFwd = ProjectConfigPath $ "a/b.config" :| []
-- >>> let abBwd = ProjectConfigPath $ "a\\b.config" :| []
-- >>> compare abFwd abBwd
-- EQ
```

## Changes in `cabal-testsuite`

### Reading Expected Multiline Strings Verbatim

With `ghc-9.12.1` adding `-XMultilineStrings`, writing multiline string
expectations for `cabal-testsuite/PackageTests/**/*.test.hs` test scripts might
be have been easier but for a catch. We run these tests with older `GHC`
versions so would need to use `-XCPP` for those versions and the C preprocessor
does not play nicely with string gaps. While it is possible to encode a
multiline string as a single line with in embedded LF characters or by breaking
the line up arbitrarily and using `++` concatenation or by calling unlines on a
list of lines, string gaps are the multiline strings of Haskell prior to
`-XMultilineStrings`.

To avoid these problems and for the convenience of pasting the expected value
verbatim into a file, `readFileVerbatim` can read the expected multiline output
for tests from a text file. This has the same implementation as `readFile` from
the `strict-io` package to avoid problems at cleanup.

```
Warning: Windows file locking hack: hit the retry limit 3 while trying to remove
C:\Users\<username>\AppData\Local\Temp\cabal-testsuite-8376
cabal.test.hs:
C:\Users\<username>\AppData\Local\Temp\cabal-testsuite-8376\errors.expect.txt: removePathForcibly:DeleteFile
"\\\\?\\C:\\Users\\<username>\\AppData\\Local\\Temp\\cabal-testsuite-8376\\errors.expect.txt":
permission denied (The process cannot access the file because it is being used by another process.)
```

The other process accessing the file is `C:\WINDOWS\System32\svchost.exe`
running a `QueryDirectory` event and this problem only occurs when the test
fails.

### Hidden Actual Value Modification

The `assertOutputContains` function was modifying the actual value (the test
output) with `concatOutput` before checking if it contained the expected value.
This function, now renamed as `lineBreaksToSpaces`, would remove CR values and
convert LF values to spaces.

```haskell
-- | Replace line breaks with spaces, correctly handling @"\\r\\n"@.
--
-- >>> lineBreaksToSpaces "foo\nbar\r\nbaz"
-- "foo bar baz"
--
-- >>> lineBreaksToSpaces "foo\nbar\r\nbaz\n"
-- "foo bar baz"
--
-- >>> lineBreaksToSpaces "\nfoo\nbar\r\nbaz\n"
-- " foo bar baz"
lineBreaksToSpaces :: String -> String
```

With this set up, false positives were possible. An expected value using string
gaps and spaces would match a `concatOutput` modified actual value of
"foo_bar_baz", where '_' was any of space, LF or CRLF in the unmodified actual
value. The latter two are false positive matches.

```haskell
let expect = "foo \
\bar \
\baz"
```

False negatives were also possible. An expected value set up using string gaps
with LF characters or with `-XMultilineStrings` wouldn't match an actual value
of "foo_bar_baz", where '_' was either LF or CRLF because these characters had
been replaced by spaces in the actual value, modified before the comparison.

```haskell
let expect = "foo\n\
\bar\n\
\baz"
```

```haskell
{-# LANGUAGE MultilineStrings #-}

let expect = """
foo
bar
baz
"""
```

We had these problems:

1. The actual value was changed before comparison and this change was not visible.
2. The expected value was not changed in the same way as the actual value. This
made it possible for equal values to become unequal (false negatives) and for
unequal values to become equal (false positives).

### Explicit Changes and Visible Line Delimiters

To fix these problems, an added `assertOn` function takes a `NeedleHaystack`
configuration for how the search is made, what to expect (to find the expected
value or not) and how to display the expected and actual values.

A pilcrow ¶ is often used to visibly display line endings but our terminal
output is restricted to ASCII so lines are delimited between `^` and `$`
markers. The needle (the expected output fragment) is shown annotated this way
and the haystack (the actual output) can optionally be shown this way too.

We can now implement `assertOutputContains` by calling `assertOn`:

```diff
assertOutputContains :: MonadIO m => WithCallStack (String -> Result -> m ())
- assertOutputContains needle result =
- withFrozenCallStack $
- unless (needle `isInfixOf` (concatOutput output)) $
- assertFailure $ " expected: " ++ needle
- where output = resultOutput result
+ assertOutputContains = assertOn
+ needleHaystack
+ {txHaystack =
+ TxContains
+ { txBwd = delimitLines
+ , txFwd = encodeLf
+ }
+ }
```

This is still a lenient match, allowing LF to match CRLF, but `encodeLf` doesn't
replace LF with spaces like `concatOutput` (`lineBreaksToSpaces`) did:

```haskell
-- | Replace line CRLF line breaks with LF line breaks.
--
-- >>> encodeLf "foo\nbar\r\nbaz"
-- "foo\nbar\nbaz"
--
-- >>> encodeLf "foo\nbar\r\nbaz\n"
-- "foo\nbar\nbaz\n"
--
-- >>> encodeLf "\nfoo\nbar\r\nbaz\n"
-- "\nfoo\nbar\nbaz\n"
--
-- >>> encodeLf "\n\n\n"
-- "\n\n\n"
encodeLf :: String -> String
```

If you choose to display the actual value by setting
`NeedleHaystack{displayHaystack = True}` then its lines will be delimited.

```haskell
-- | Mark lines with visible delimiters, @^@ at the start and @$@ at the end.
--
-- >>> delimitLines ""
-- "^$"
--
-- >>> delimitLines "\n"
-- "^$\n"
--
-- >>> delimitLines "\n\n"
-- "^$\n^$\n"
--
-- >>> delimitLines "\n\n\n"
-- "^$\n^$\n^$\n"
--
-- >>> delimitLines $ encodeLf "foo\nbar\r\nbaz"
-- "^foo$\n^bar$\n^baz$"
--
-- >>> delimitLines $ encodeLf "foo\nbar\r\nbaz\n"
-- "^foo$\n^bar$\n^baz$\n"
--
-- >>> delimitLines $ encodeLf "\nfoo\nbar\r\nbaz\n"
-- "^$\n^foo$\n^bar$\n^baz$\n"
delimitLines:: String -> String
```

With `assertOn`, supplying string transformation to both the needle and haystack
before comparison and before display can help find out why an expected value is
or isn't found in the test output.

0 comments on commit 14850de

Please sign in to comment.