Skip to content

Implement assert matches with assert on #10735

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 27, 2025

Conversation

philderbeast
Copy link
Collaborator

A follow on from #10646 that I had to rebase this on top of changes from #10647 that added assertOutputMatches and assertOutputDoesNotMatch to cabal-testsuite/src/Test/Cabal/Prelude.hs. At that time I made this small change but saw that assertOutputMatches and assertOutputDoesNotMatch could be implemented with assertOn. That what this pull request does.

-    unless (concatOutput output =~ regex) $
+    unless (encodeLf output =~ regex) $

Template B: This PR does not modify behaviour or interface

  • Patches conform to the coding conventions.
  • Is this a PR that fixes CI? If so, it will need to be backported to older cabal release branches (ask maintainers for directions).

QA Notes

I've moved tests to cabal-testsuite/AssertTests/cabal.test.hs that are only flexing the assert* functions and are not truly package tests. These tests pass but you'll want to see them fail too to see that the output is as expected. To do that you'll need to make a change and rerun the test:

$ git diff
diff --git a/cabal-testsuite/AssertTests/cabal.test.hs b/cabal-testsuite/AssertTests/cabal.test.hs
index d819c99cc..b44acd2e4 100644
--- a/cabal-testsuite/AssertTests/cabal.test.hs
+++ b/cabal-testsuite/AssertTests/cabal.test.hs
@@ -29,7 +29,7 @@ main = cabalTest . recordMode RecordMarked $ do
   assertOutputMatches " errors " out
   assertOutputDoesNotMatch " error " out
 
-  assertOutputMatches "[[:space:]]+errors[[:space:]]+" out
+  assertOutputMatches "[[:space:]]+ewwows[[:space:]]+" out
   assertOutputDoesNotMatch "[[:space:]]+error[[:space:]]+" out
 
   log "Pseudo multiline string marking:"
$ cabal run cabal-testsuite:cabal-tests -- --with-cabal=./dist-newstyle/build/x86_64-linux/ghc-9.10.1/cabal-install-3.15.0.0/x/cabal/build/cabal/cabal cabal-testsuite/AssertTests/cabal.test.hs
...
# STDERR:
cabal.test.hs: expected:
regex match with '[[:space:]]+ewwows[[:space:]]+'
CallStack (from HasCallStack):
  assertOn, called at src/Test/Cabal/Prelude.hs:835:23 in cabal-testsuite-3-inplace:Test.Cabal.Prelude
  assertOutputMatches, called at cabal-testsuite/AssertTests/cabal.test.hs:32:3 in main:Main

Copy link
Collaborator

@9999years 9999years left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is good. Longer-term, I want better exception-throwing infrastructure that makes it possible to use the normal expect-test and output-marker machinery for these tests.

@philderbeast
Copy link
Collaborator Author

the normal expect-test and output-marker machinery for these tests.

Thanks for the approval @9999years. Could you please link to or describe what you mean by the above?

@9999years
Copy link
Collaborator

@philderbeast I'll put some more details here soon, but here's the general idea:
#10747

@philderbeast philderbeast force-pushed the test/assert-output-matches branch from ff435b7 to 9761855 Compare January 16, 2025 18:52
@philderbeast philderbeast requested a review from ffaf1 January 16, 2025 18:52
@philderbeast philderbeast force-pushed the test/assert-output-matches branch 2 times, most recently from 9f19760 to bfcac48 Compare January 24, 2025 11:24
@philderbeast philderbeast force-pushed the test/assert-output-matches branch from bfcac48 to e85edd8 Compare February 2, 2025 01:58
@philderbeast philderbeast force-pushed the test/assert-output-matches branch from e85edd8 to 45af92c Compare April 19, 2025 12:44
@Mikolaj
Copy link
Member

Mikolaj commented Apr 24, 2025

@philderbeast: could you please restart CI and set the merge label? thank you for the PR!

@philderbeast philderbeast force-pushed the test/assert-output-matches branch from 45af92c to 58bd2ac Compare April 25, 2025 10:34
- Add NeedleHaystackCompare
- Add tests of assertions
- Quote regex pattern
- Add regex tests
- Add assertOutputMatches tests
- Add tests using POSIX character classes
- Rename to TxFwdBwd
- Add isInfixOf predicate to assertOn calls
@philderbeast philderbeast force-pushed the test/assert-output-matches branch from 58bd2ac to b2d1fcc Compare April 25, 2025 10:35
@mergify mergify bot added the ready and waiting Mergify is waiting out the cooldown period label Apr 25, 2025
@philderbeast
Copy link
Collaborator Author

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

@mergify mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Apr 27, 2025
@mergify mergify bot merged commit 7e0f040 into haskell:master Apr 27, 2025
52 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days merge+no rebase ready and waiting Mergify is waiting out the cooldown period
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants