From 27073a6683bbdbcf4fe11c9ed9d3ee4c4d01a866 Mon Sep 17 00:00:00 2001 From: Greg Pfeil Date: Thu, 3 Oct 2024 00:42:45 -0600 Subject: [PATCH 1/8] Added support for known failures in transcripts Fixes #5350. --- unison-cli/src/Unison/Codebase/Transcript.hs | 8 ++-- .../src/Unison/Codebase/Transcript/Parser.hs | 7 +++- .../src/Unison/Codebase/Transcript/Runner.hs | 39 ++++++++++--------- 3 files changed, 30 insertions(+), 24 deletions(-) diff --git a/unison-cli/src/Unison/Codebase/Transcript.hs b/unison-cli/src/Unison/Codebase/Transcript.hs index bd5bbd058f..6248d4bda2 100644 --- a/unison-cli/src/Unison/Codebase/Transcript.hs +++ b/unison-cli/src/Unison/Codebase/Transcript.hs @@ -2,7 +2,7 @@ -- | The data model for Unison transcripts. module Unison.Codebase.Transcript - ( ExpectingError, + ( Result (..), ScratchFileName, Hidden (..), UcmLine (..), @@ -19,7 +19,7 @@ import Unison.Core.Project (ProjectBranchName, ProjectName) import Unison.Prelude import Unison.Project (ProjectAndBranch) -type ExpectingError = Bool +data Result = Success | Error | Failure type ScratchFileName = Text @@ -45,6 +45,6 @@ pattern CMarkCodeBlock pos info body = CMark.Node pos (CMark.CODE_BLOCK info bod type Stanza = Either CMark.Node ProcessedBlock data ProcessedBlock - = Ucm Hidden ExpectingError [UcmLine] - | Unison Hidden ExpectingError (Maybe ScratchFileName) Text + = Ucm Hidden Result [UcmLine] + | Unison Hidden Result (Maybe ScratchFileName) Text | API [APIRequest] diff --git a/unison-cli/src/Unison/Codebase/Transcript/Parser.hs b/unison-cli/src/Unison/Codebase/Transcript/Parser.hs index d19951de0c..bb66292579 100644 --- a/unison-cli/src/Unison/Codebase/Transcript/Parser.hs +++ b/unison-cli/src/Unison/Codebase/Transcript/Parser.hs @@ -142,8 +142,11 @@ hidden = <|> (HideOutput <$ word ":hide") <|> pure Shown -expectingError :: P ExpectingError -expectingError = isJust <$> optional (word ":error") +expectingError :: P Result +expectingError = + (Error <$ word ":error") + <|> (Failure <$ word ":failure") + <|> pure Success untilSpace1 :: P Text untilSpace1 = P.takeWhile1P Nothing (not . Char.isSpace) diff --git a/unison-cli/src/Unison/Codebase/Transcript/Runner.hs b/unison-cli/src/Unison/Codebase/Transcript/Runner.hs index 6a91e069a9..f51320242c 100644 --- a/unison-cli/src/Unison/Codebase/Transcript/Runner.hs +++ b/unison-cli/src/Unison/Codebase/Transcript/Runner.hs @@ -159,7 +159,7 @@ run isTest verbosity dir stanzas codebase runtime sbRuntime nRuntime ucmVersion unisonFiles <- newIORef Map.empty out <- newIORef mempty hidden <- newIORef Shown - allowErrors <- newIORef False + allowErrors <- newIORef Success hasErrors <- newIORef False mStanza <- newIORef Nothing traverse_ (atomically . Q.enqueue inputQueue) (stanzas `zip` (Just <$> [1 :: Int ..])) @@ -265,18 +265,17 @@ run isTest verbosity dir stanzas codebase runtime sbRuntime nRuntime ucmVersion Left msg -> do liftIO $ writeIORef hasErrors True liftIO (readIORef allowErrors) >>= \case - True -> do - liftIO (output . Pretty.toPlain terminalWidth $ ("\n" <> msg <> "\n")) + Success -> liftIO . dieWithMsg $ Pretty.toPlain terminalWidth msg + _ -> do + liftIO . output . Pretty.toPlain terminalWidth $ "\n" <> msg <> "\n" awaitInput - False -> do - liftIO (dieWithMsg $ Pretty.toPlain terminalWidth msg) -- No input received from this line, try again. Right Nothing -> awaitInput Right (Just (_expandedArgs, input)) -> pure $ Right input Nothing -> do liftIO (dieUnexpectedSuccess) liftIO (writeIORef hidden Shown) - liftIO (writeIORef allowErrors False) + liftIO (writeIORef allowErrors Success) maybeStanza <- atomically (Q.tryDequeue inputQueue) _ <- liftIO (writeIORef mStanza maybeStanza) case maybeStanza of @@ -365,10 +364,9 @@ run isTest verbosity dir stanzas codebase runtime sbRuntime nRuntime ucmVersion errOk <- readIORef allowErrors let rendered = Pretty.toPlain terminalWidth (Pretty.border 2 msg) output rendered - when (Output.isFailure o) $ - if errOk - then writeIORef hasErrors True - else dieWithMsg rendered + when (Output.isFailure o) case errOk of + Success -> dieWithMsg rendered + _ -> writeIORef hasErrors True printNumbered :: Output.NumberedOutput -> IO Output.NumberedArgs printNumbered o = do @@ -376,10 +374,9 @@ run isTest verbosity dir stanzas codebase runtime sbRuntime nRuntime ucmVersion errOk <- readIORef allowErrors let rendered = Pretty.toPlain terminalWidth (Pretty.border 2 msg) output rendered - when (Output.isNumberedFailure o) $ - if errOk - then writeIORef hasErrors True - else dieWithMsg rendered + when (Output.isNumberedFailure o) case errOk of + Success -> dieWithMsg rendered + _ -> writeIORef hasErrors True pure numberedArgs -- Looks at the current stanza and decides if it is contained in the @@ -404,10 +401,16 @@ run isTest verbosity dir stanzas codebase runtime sbRuntime nRuntime ucmVersion dieUnexpectedSuccess = do errOk <- readIORef allowErrors hasErr <- readIORef hasErrors - when (errOk && not hasErr) $ do - output "\n```\n\n" - appendFailingStanza - transcriptFailure out "The transcript was expecting an error in the stanza above, but did not encounter one." + case (errOk, hasErr) of + (Error, False) -> do + output "\n```\n\n" + appendFailingStanza + transcriptFailure out "The transcript was expecting an error in the stanza above, but did not encounter one." + (Failure, False) -> do + output "\n```\n\n" + appendFailingStanza + transcriptFailure out "The stanza above is now passing! Please remove `:failure` from it." + (_, _) -> pure () authenticatedHTTPClient <- AuthN.newAuthenticatedHTTPClient tokenProvider ucmVersion From dc2db62caa336bbc44956632ce47529e8d586be6 Mon Sep 17 00:00:00 2001 From: Greg Pfeil Date: Thu, 3 Oct 2024 09:30:17 -0600 Subject: [PATCH 2/8] =?UTF-8?q?Add=20the=20first=20=E2=80=9Cknown=20failur?= =?UTF-8?q?e=E2=80=9D=20transcript?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Shows that #5337 is _not_ fixed. --- unison-src/transcripts/fix5337.md | 14 +++++++++++ unison-src/transcripts/fix5337.output.md | 32 ++++++++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 unison-src/transcripts/fix5337.md create mode 100644 unison-src/transcripts/fix5337.output.md diff --git a/unison-src/transcripts/fix5337.md b/unison-src/transcripts/fix5337.md new file mode 100644 index 0000000000..54df389bc5 --- /dev/null +++ b/unison-src/transcripts/fix5337.md @@ -0,0 +1,14 @@ +```ucm +scratch/main> builtins.mergeio +``` + +The following `Doc` fails to typecheck with `ucm` `0.5.26`: + +```unison:failure +testDoc : Doc2 +testDoc = {{ + key: '{{ docWord "value" }}'. +}} +``` + +The same code typechecks ok with `0.5.25`. diff --git a/unison-src/transcripts/fix5337.output.md b/unison-src/transcripts/fix5337.output.md new file mode 100644 index 0000000000..2b4070730a --- /dev/null +++ b/unison-src/transcripts/fix5337.output.md @@ -0,0 +1,32 @@ +``` ucm +scratch/main> builtins.mergeio + + Done. + +``` +The following `Doc` fails to typecheck with `ucm` `0.5.26`: + +``` unison +testDoc : Doc2 +testDoc = {{ + key: '{{ docWord "value" }}'. +}} +``` + +``` ucm + + Loading changes detected in scratch.u. + + I got confused here: + + 3 | key: '{{ docWord "value" }}'. + + + I was surprised to find a . here. + I was expecting one of these instead: + + * end of input + +``` +The same code typechecks ok with `0.5.25`. + From 728c2a3bf58c3d393342915ee5f8dd65f9eccfc2 Mon Sep 17 00:00:00 2001 From: Greg Pfeil Date: Thu, 3 Oct 2024 09:45:28 -0600 Subject: [PATCH 3/8] Add an `:incorrect` transcript tag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is basically the opposite of `:failure` – it indicates that a successful result is incorrect. The correct result may either be an error (which will be caught by the transcript runner) or a different result then what is currently expected (which won’t be caught by the runner, but the appearance of `:incorrect` conveys to the programmer that the output.md diff may be an improvement from the previous state). --- unison-cli/src/Unison/Codebase/Transcript.hs | 2 +- unison-cli/src/Unison/Codebase/Transcript/Parser.hs | 1 + unison-cli/src/Unison/Codebase/Transcript/Runner.hs | 10 ++++++++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/unison-cli/src/Unison/Codebase/Transcript.hs b/unison-cli/src/Unison/Codebase/Transcript.hs index 6248d4bda2..3b48d32c05 100644 --- a/unison-cli/src/Unison/Codebase/Transcript.hs +++ b/unison-cli/src/Unison/Codebase/Transcript.hs @@ -19,7 +19,7 @@ import Unison.Core.Project (ProjectBranchName, ProjectName) import Unison.Prelude import Unison.Project (ProjectAndBranch) -data Result = Success | Error | Failure +data Result = Success | Incorrect | Error | Failure type ScratchFileName = Text diff --git a/unison-cli/src/Unison/Codebase/Transcript/Parser.hs b/unison-cli/src/Unison/Codebase/Transcript/Parser.hs index bb66292579..1137d3fb6a 100644 --- a/unison-cli/src/Unison/Codebase/Transcript/Parser.hs +++ b/unison-cli/src/Unison/Codebase/Transcript/Parser.hs @@ -146,6 +146,7 @@ expectingError :: P Result expectingError = (Error <$ word ":error") <|> (Failure <$ word ":failure") + <|> (Incorrect <$ word ":incorrect") <|> pure Success untilSpace1 :: P Text diff --git a/unison-cli/src/Unison/Codebase/Transcript/Runner.hs b/unison-cli/src/Unison/Codebase/Transcript/Runner.hs index f51320242c..65e6532fa2 100644 --- a/unison-cli/src/Unison/Codebase/Transcript/Runner.hs +++ b/unison-cli/src/Unison/Codebase/Transcript/Runner.hs @@ -266,6 +266,14 @@ run isTest verbosity dir stanzas codebase runtime sbRuntime nRuntime ucmVersion liftIO $ writeIORef hasErrors True liftIO (readIORef allowErrors) >>= \case Success -> liftIO . dieWithMsg $ Pretty.toPlain terminalWidth msg + Incorrect -> + liftIO . dieWithMsg . Pretty.toPlain terminalWidth $ + "The stanza above previously had an incorrect successful result, but now fails with" + <> "\n" + <> Pretty.border 2 msg + <> "\n" + <> "if this is the expected result, replace `:incorrect` with `:error`, otherwise " + <> "change `:incorrect` to `:failure`." _ -> do liftIO . output . Pretty.toPlain terminalWidth $ "\n" <> msg <> "\n" awaitInput @@ -366,6 +374,7 @@ run isTest verbosity dir stanzas codebase runtime sbRuntime nRuntime ucmVersion output rendered when (Output.isFailure o) case errOk of Success -> dieWithMsg rendered + Incorrect -> dieWithMsg rendered _ -> writeIORef hasErrors True printNumbered :: Output.NumberedOutput -> IO Output.NumberedArgs @@ -376,6 +385,7 @@ run isTest verbosity dir stanzas codebase runtime sbRuntime nRuntime ucmVersion output rendered when (Output.isNumberedFailure o) case errOk of Success -> dieWithMsg rendered + Incorrect -> dieWithMsg rendered _ -> writeIORef hasErrors True pure numberedArgs From 56a2f5cf3ffb0566c4389499e1b5668c8b99dd47 Mon Sep 17 00:00:00 2001 From: Greg Pfeil Date: Thu, 3 Oct 2024 10:47:01 -0600 Subject: [PATCH 4/8] Add first `:incorrect` transcript Shows that #5178 is _not_ fixed. --- unison-src/transcripts-using-base/fix5178.md | 20 +++++++++ .../transcripts-using-base/fix5178.output.md | 45 +++++++++++++++++++ 2 files changed, 65 insertions(+) create mode 100644 unison-src/transcripts-using-base/fix5178.md create mode 100644 unison-src/transcripts-using-base/fix5178.output.md diff --git a/unison-src/transcripts-using-base/fix5178.md b/unison-src/transcripts-using-base/fix5178.md new file mode 100644 index 0000000000..62f8a2b6ea --- /dev/null +++ b/unison-src/transcripts-using-base/fix5178.md @@ -0,0 +1,20 @@ +```unison +foo = {{ +@source{Stream.emit} +}} +``` + +```ucm +scratch/main> add +``` + +Viewing `foo` via `scratch/main> ui` shows the correct source, but `display foo` gives us an error message (but not an error – this is incorrectly considered a successful result) + +I think there are two separate issues here: + +1. this message should be considered an error, not success; and +2. this should actually work like `ui` and give us the source of the ability member, not complain about there being no such term in the codebase. + +```ucm:incorrect +scratch/main> display foo +``` diff --git a/unison-src/transcripts-using-base/fix5178.output.md b/unison-src/transcripts-using-base/fix5178.output.md new file mode 100644 index 0000000000..7b80a26cee --- /dev/null +++ b/unison-src/transcripts-using-base/fix5178.output.md @@ -0,0 +1,45 @@ +``` unison +foo = {{ +@source{Stream.emit} +}} +``` + +``` ucm + + Loading changes detected in scratch.u. + + I found and typechecked these definitions in scratch.u. If you + do an `add` or `update`, here's how your codebase would + change: + + ⍟ These new definitions are ok to `add`: + + foo : Doc2 + +``` +``` ucm +scratch/main> add + + ⍟ I've added these definitions: + + foo : Doc2 + +``` +Viewing `foo` via `scratch/main> ui` shows the correct source, but `display foo` gives us an error message (but not an error – this is incorrectly considered a successful result) + +I think there are two separate issues here: + +1. this message should be considered an error, not success; and +2. this should actually work like `ui` and give us the source of the ability member, not complain about there being no such term in the codebase. + +``` ucm +scratch/main> display foo + + -- The name #rfi1v9429f is assigned to the reference + ShortHash {prefix = + "rfi1v9429f9qluv533l2iba77aadttilrpmnhljfapfnfa6sru2nr8ibpqvib9nc4s4nb9s1as45upsfqfqe6ivqi2p82b2vd866it8", + cycle = Nothing, cid = Nothing}, which is missing from the + codebase. + Tip: You might need to repair the codebase manually. + +``` From b016af08ce3ae3854aa337a78b9d3367b35a44c5 Mon Sep 17 00:00:00 2001 From: Greg Pfeil Date: Fri, 6 Dec 2024 10:19:11 -0700 Subject: [PATCH 5/8] Fix some outdated transcripts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These aren’t run by check.sh, so I missed them before pushing. --- .../builtin-tests/interpreter-tests.output.md | 2 +- unison-src/builtin-tests/jit-tests.output.md | 30 ++++++++++++------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/unison-src/builtin-tests/interpreter-tests.output.md b/unison-src/builtin-tests/interpreter-tests.output.md index 8f313d114f..87de1b4977 100644 --- a/unison-src/builtin-tests/interpreter-tests.output.md +++ b/unison-src/builtin-tests/interpreter-tests.output.md @@ -4,7 +4,7 @@ If you want to add or update tests, you can create a branch of that project, and Before merging the PR on Github, we'll merge your branch on Share and restore `runtime_tests_version` to /main or maybe a release. -``` ucm :hide:error +``` ucm :hide :error scratch/main> this is a hack to trigger an error, in order to swallow any error on the next line. scratch/main> we delete the project to avoid any merge conflicts or complaints from ucm. diff --git a/unison-src/builtin-tests/jit-tests.output.md b/unison-src/builtin-tests/jit-tests.output.md index 616d2d5d9c..94225ebd14 100644 --- a/unison-src/builtin-tests/jit-tests.output.md +++ b/unison-src/builtin-tests/jit-tests.output.md @@ -4,6 +4,18 @@ If you want to add or update tests, you can create a branch of that project, and Before merging the PR on Github, we'll merge your branch on Share and restore `runtime_tests_version` to /main or maybe a release. +``` ucm :hide :error +scratch/main> this is a hack to trigger an error, in order to swallow any error on the next line. + +scratch/main> we delete the project to avoid any merge conflicts or complaints from ucm. + +scratch/main> delete.project runtime-tests +``` + +``` ucm :hide +scratch/main> clone @unison/runtime-tests/releases/0.0.1 runtime-tests/selected +``` + ``` ucm runtime-tests/selected> run.native tests @@ -12,8 +24,8 @@ runtime-tests/selected> run.native tests runtime-tests/selected> run.native tests.jit.only () - ``` + Per Dan: It's testing a flaw in how we were sending code from a scratch file to the native runtime, when that happened multiple times. Related to the verifiable refs and recursive functions. @@ -27,19 +39,18 @@ foo = do go 1000 ``` -``` ucm - +``` ucm :added-by-ucm Loading changes detected in scratch.u. I found and typechecked these definitions in scratch.u. If you do an `add` or `update`, here's how your codebase would change: - + ⍟ These new definitions are ok to `add`: foo : '{Exception} () - ``` + ``` ucm scratch/main> run.native foo @@ -48,20 +59,19 @@ scratch/main> run.native foo scratch/main> run.native foo () - ``` + This can also only be tested by separately running this test, because it is exercising the protocol that ucm uses to talk to the jit during an exception. -``` ucm +``` ucm :error runtime-tests/selected> run.native testBug 💔💥 - + I've encountered a call to builtin.bug with the following value: - - "testing" + "testing" ``` From 49a0cae72575ae44087d23b3887fd18e58cdcf86 Mon Sep 17 00:00:00 2001 From: Greg Pfeil Date: Mon, 9 Dec 2024 23:33:56 -0700 Subject: [PATCH 6/8] Replace `:hide:all` with `:hide-all` The former was confusing because `:hide` and `:all` are not actually separate tags. --- unison-cli/src/Unison/Codebase/Transcript/Parser.hs | 4 ++-- unison-src/transcripts/alias-many.md | 2 +- .../transcripts/errors/info-string-parse-error.output.md | 2 +- unison-src/transcripts/errors/missing-result-typed.md | 4 ++-- .../transcripts/errors/missing-result-typed.output.md | 4 ++-- unison-src/transcripts/errors/missing-result.md | 4 ++-- unison-src/transcripts/errors/missing-result.output.md | 4 ++-- unison-src/transcripts/errors/ucm-hide-all-error.md | 4 ++-- unison-src/transcripts/errors/ucm-hide-all-error.output.md | 4 ++-- unison-src/transcripts/errors/ucm-hide-all.md | 4 ++-- unison-src/transcripts/errors/ucm-hide-all.output.md | 4 ++-- unison-src/transcripts/errors/unison-hide-all-error.md | 4 ++-- .../transcripts/errors/unison-hide-all-error.output.md | 4 ++-- unison-src/transcripts/errors/unison-hide-all.md | 4 ++-- unison-src/transcripts/errors/unison-hide-all.output.md | 4 ++-- unison-src/transcripts/fix2840.md | 2 +- unison-src/transcripts/hello.md | 4 ++-- unison-src/transcripts/hello.output.md | 2 +- unison-src/transcripts/merge.md | 6 +++--- unison-src/transcripts/no-hash-in-term-declaration.md | 2 +- 20 files changed, 36 insertions(+), 36 deletions(-) diff --git a/unison-cli/src/Unison/Codebase/Transcript/Parser.hs b/unison-cli/src/Unison/Codebase/Transcript/Parser.hs index e2f54065dd..cc335e9f7c 100644 --- a/unison-cli/src/Unison/Codebase/Transcript/Parser.hs +++ b/unison-cli/src/Unison/Codebase/Transcript/Parser.hs @@ -145,13 +145,13 @@ nonNewlineSpaces = void $ P.takeWhileP Nothing (\ch -> ch == ' ' || ch == '\t') formatHidden :: Hidden -> Maybe Text formatHidden = \case - HideAll -> pure ":hide:all" + HideAll -> pure ":hide-all" HideOutput -> pure ":hide" Shown -> Nothing hidden :: P Hidden hidden = - (HideAll <$ word ":hide:all") + (HideAll <$ word ":hide-all") <|> (HideOutput <$ word ":hide") <|> pure Shown diff --git a/unison-src/transcripts/alias-many.md b/unison-src/transcripts/alias-many.md index 4cc88d489a..e693e50a5b 100644 --- a/unison-src/transcripts/alias-many.md +++ b/unison-src/transcripts/alias-many.md @@ -1,7 +1,7 @@ ``` ucm :hide scratch/main> builtins.merge lib.builtins ``` -``` unison :hide:all +``` unison :hide-all List.adjacentPairs : [a] -> [(a, a)] List.adjacentPairs as = go xs acc = diff --git a/unison-src/transcripts/errors/info-string-parse-error.output.md b/unison-src/transcripts/errors/info-string-parse-error.output.md index 41dfb5e229..3ef6a22af4 100644 --- a/unison-src/transcripts/errors/info-string-parse-error.output.md +++ b/unison-src/transcripts/errors/info-string-parse-error.output.md @@ -3,4 +3,4 @@ 1 | ``` ucm :hode | ^ unexpected ':' -expecting ":added-by-ucm", ":bug", ":error", ":hide", ":hide:all", or newline +expecting ":added-by-ucm", ":bug", ":error", ":hide", ":hide-all", or newline diff --git a/unison-src/transcripts/errors/missing-result-typed.md b/unison-src/transcripts/errors/missing-result-typed.md index 0e6e52b806..70949bec81 100644 --- a/unison-src/transcripts/errors/missing-result-typed.md +++ b/unison-src/transcripts/errors/missing-result-typed.md @@ -1,6 +1,6 @@ ### Transcript parser hidden errors -When an error is encountered in a `unison :hide:all` block +When an error is encountered in a `unison :hide-all` block then the transcript parser should print the stanza and surface a helpful message. @@ -8,7 +8,7 @@ and surface a helpful message. scratch/main> builtins.merge ``` -``` unison :hide:all +``` unison :hide-all a : Nat a = b = 24 diff --git a/unison-src/transcripts/errors/missing-result-typed.output.md b/unison-src/transcripts/errors/missing-result-typed.output.md index 87c2308bec..f28268036c 100644 --- a/unison-src/transcripts/errors/missing-result-typed.output.md +++ b/unison-src/transcripts/errors/missing-result-typed.output.md @@ -1,6 +1,6 @@ ### Transcript parser hidden errors -When an error is encountered in a `unison :hide:all` block +When an error is encountered in a `unison :hide-all` block then the transcript parser should print the stanza and surface a helpful message. @@ -8,7 +8,7 @@ and surface a helpful message. scratch/main> builtins.merge ``` -``` unison :hide:all +``` unison :hide-all a : Nat a = b = 24 diff --git a/unison-src/transcripts/errors/missing-result.md b/unison-src/transcripts/errors/missing-result.md index f177ee81c8..a94c3bb3c5 100644 --- a/unison-src/transcripts/errors/missing-result.md +++ b/unison-src/transcripts/errors/missing-result.md @@ -1,10 +1,10 @@ ### Transcript parser hidden errors -When an error is encountered in a `unison :hide:all` block +When an error is encountered in a `unison :hide-all` block then the transcript parser should print the stanza and surface a helpful message. -``` unison :hide:all +``` unison :hide-all x = y = 24 ``` diff --git a/unison-src/transcripts/errors/missing-result.output.md b/unison-src/transcripts/errors/missing-result.output.md index fb0ab98c9f..faf91774a6 100644 --- a/unison-src/transcripts/errors/missing-result.output.md +++ b/unison-src/transcripts/errors/missing-result.output.md @@ -1,10 +1,10 @@ ### Transcript parser hidden errors -When an error is encountered in a `unison :hide:all` block +When an error is encountered in a `unison :hide-all` block then the transcript parser should print the stanza and surface a helpful message. -``` unison :hide:all +``` unison :hide-all x = y = 24 ``` diff --git a/unison-src/transcripts/errors/ucm-hide-all-error.md b/unison-src/transcripts/errors/ucm-hide-all-error.md index 7444155923..7a56730f69 100644 --- a/unison-src/transcripts/errors/ucm-hide-all-error.md +++ b/unison-src/transcripts/errors/ucm-hide-all-error.md @@ -2,10 +2,10 @@ Dangerous scary words! -When an expected error is not encountered in a `ucm :hide:all` block +When an expected error is not encountered in a `ucm :hide-all` block then the transcript parser should print the stanza and surface a helpful message. -``` ucm :hide:all:error +``` ucm :hide-all :error scratch/main> history ``` diff --git a/unison-src/transcripts/errors/ucm-hide-all-error.output.md b/unison-src/transcripts/errors/ucm-hide-all-error.output.md index 35770aff86..6f7c903cbd 100644 --- a/unison-src/transcripts/errors/ucm-hide-all-error.output.md +++ b/unison-src/transcripts/errors/ucm-hide-all-error.output.md @@ -2,11 +2,11 @@ Dangerous scary words\! -When an expected error is not encountered in a `ucm :hide:all` block +When an expected error is not encountered in a `ucm :hide-all` block then the transcript parser should print the stanza and surface a helpful message. -``` ucm :hide:all :error +``` ucm :hide-all :error scratch/main> history ``` diff --git a/unison-src/transcripts/errors/ucm-hide-all.md b/unison-src/transcripts/errors/ucm-hide-all.md index cb79d26753..a3e6d3443f 100644 --- a/unison-src/transcripts/errors/ucm-hide-all.md +++ b/unison-src/transcripts/errors/ucm-hide-all.md @@ -2,10 +2,10 @@ Dangerous scary words! -When an error is encountered in a `ucm :hide:all` block +When an error is encountered in a `ucm :hide-all` block then the transcript parser should print the stanza and surface a helpful message. -``` ucm :hide:all +``` ucm :hide-all scratch/main> move.namespace foo bar ``` diff --git a/unison-src/transcripts/errors/ucm-hide-all.output.md b/unison-src/transcripts/errors/ucm-hide-all.output.md index 2753dd7f11..fc6d21cbc6 100644 --- a/unison-src/transcripts/errors/ucm-hide-all.output.md +++ b/unison-src/transcripts/errors/ucm-hide-all.output.md @@ -2,11 +2,11 @@ Dangerous scary words\! -When an error is encountered in a `ucm :hide:all` block +When an error is encountered in a `ucm :hide-all` block then the transcript parser should print the stanza and surface a helpful message. -``` ucm :hide:all +``` ucm :hide-all scratch/main> move.namespace foo bar ``` diff --git a/unison-src/transcripts/errors/unison-hide-all-error.md b/unison-src/transcripts/errors/unison-hide-all-error.md index e35de94e1d..ca2bd023ba 100644 --- a/unison-src/transcripts/errors/unison-hide-all-error.md +++ b/unison-src/transcripts/errors/unison-hide-all-error.md @@ -1,9 +1,9 @@ ### Transcript parser hidden errors -When an expected error is not encountered in a `unison :hide:all:error` block +When an expected error is not encountered in a `unison :hide-all :error` block then the transcript parser should print the stanza and surface a helpful message. -``` unison :hide:all:error +``` unison :hide-all :error myVal = 3 ``` diff --git a/unison-src/transcripts/errors/unison-hide-all-error.output.md b/unison-src/transcripts/errors/unison-hide-all-error.output.md index 94aeb9cf66..6205069903 100644 --- a/unison-src/transcripts/errors/unison-hide-all-error.output.md +++ b/unison-src/transcripts/errors/unison-hide-all-error.output.md @@ -1,10 +1,10 @@ ### Transcript parser hidden errors -When an expected error is not encountered in a `unison :hide:all:error` block +When an expected error is not encountered in a `unison :hide-all :error` block then the transcript parser should print the stanza and surface a helpful message. -``` unison :hide:all :error +``` unison :hide-all :error myVal = 3 ``` diff --git a/unison-src/transcripts/errors/unison-hide-all.md b/unison-src/transcripts/errors/unison-hide-all.md index 48907e75e7..9288252881 100644 --- a/unison-src/transcripts/errors/unison-hide-all.md +++ b/unison-src/transcripts/errors/unison-hide-all.md @@ -1,9 +1,9 @@ ### Transcript parser hidden errors -When an error is encountered in a `unison :hide:all` block +When an error is encountered in a `unison :hide-all` block then the transcript parser should print the stanza and surface a helpful message. -``` unison :hide:all +``` unison :hide-all g 3 ``` diff --git a/unison-src/transcripts/errors/unison-hide-all.output.md b/unison-src/transcripts/errors/unison-hide-all.output.md index c27b7dd28f..89cd4724b7 100644 --- a/unison-src/transcripts/errors/unison-hide-all.output.md +++ b/unison-src/transcripts/errors/unison-hide-all.output.md @@ -1,10 +1,10 @@ ### Transcript parser hidden errors -When an error is encountered in a `unison :hide:all` block +When an error is encountered in a `unison :hide-all` block then the transcript parser should print the stanza and surface a helpful message. -``` unison :hide:all +``` unison :hide-all g 3 ``` diff --git a/unison-src/transcripts/fix2840.md b/unison-src/transcripts/fix2840.md index 6c6ac6abe9..31d4c103df 100644 --- a/unison-src/transcripts/fix2840.md +++ b/unison-src/transcripts/fix2840.md @@ -6,7 +6,7 @@ scratch/main> builtins.merge First, a few \[hidden] definitions necessary for typechecking a simple Doc2. -``` unison :hide:all +``` unison :hide-all structural type Optional a = None | Some a unique[b7a4fb87e34569319591130bf3ec6e24c9955b6a] type Doc2 diff --git a/unison-src/transcripts/hello.md b/unison-src/transcripts/hello.md index 7f5937a353..566e6b5694 100644 --- a/unison-src/transcripts/hello.md +++ b/unison-src/transcripts/hello.md @@ -52,9 +52,9 @@ This works for `ucm` blocks as well. scratch/main> rename.term x answerToUltimateQuestionOfLife ``` -Doing `unison :hide:all` hides the block altogether, both input and output - this is useful for doing behind-the-scenes control of `ucm`'s state. +Doing `unison :hide-all` hides the block altogether, both input and output - this is useful for doing behind-the-scenes control of `ucm`'s state. -``` unison :hide:all +``` unison :hide-all > [: you won't see me :] ``` diff --git a/unison-src/transcripts/hello.output.md b/unison-src/transcripts/hello.output.md index b6b1811758..9ab978d5ce 100644 --- a/unison-src/transcripts/hello.output.md +++ b/unison-src/transcripts/hello.output.md @@ -72,7 +72,7 @@ This works for `ucm` blocks as well. scratch/main> rename.term x answerToUltimateQuestionOfLife ``` -Doing `unison :hide:all` hides the block altogether, both input and output - this is useful for doing behind-the-scenes control of `ucm`'s state. +Doing `unison :hide-all` hides the block altogether, both input and output - this is useful for doing behind-the-scenes control of `ucm`'s state. ## Expecting failures diff --git a/unison-src/transcripts/merge.md b/unison-src/transcripts/merge.md index 6b759f44ce..7bbbd16cf6 100644 --- a/unison-src/transcripts/merge.md +++ b/unison-src/transcripts/merge.md @@ -870,7 +870,7 @@ scratch/alice> delete.term Foo.Bar.Baz scratch/alice> delete.term Foo.Bar.Qux ``` -``` unison :hide:all +``` unison :hide-all Foo.Bar.Baz : Nat Foo.Bar.Baz = 100 @@ -1301,7 +1301,7 @@ Alice's branch: scratch/main> branch alice ``` -``` unison :hide:all +``` unison :hide-all unique type Foo = Bar ``` @@ -1315,7 +1315,7 @@ Bob's branch: scratch/main> branch bob ``` -``` unison :hide:all +``` unison :hide-all bob : Nat bob = 101 ``` diff --git a/unison-src/transcripts/no-hash-in-term-declaration.md b/unison-src/transcripts/no-hash-in-term-declaration.md index 493c2f32ce..85ef6c0de2 100644 --- a/unison-src/transcripts/no-hash-in-term-declaration.md +++ b/unison-src/transcripts/no-hash-in-term-declaration.md @@ -2,7 +2,7 @@ There should not be hashes in the names used in term declarations, either in the type signature or the type definition. -``` unison :hide:all:error +``` unison :hide-all :error x##Nat : Int -> Int -> Boolean x##Nat = 5 ``` From de75614acb8b1170142e6863f394f13decfbd043 Mon Sep 17 00:00:00 2001 From: Greg Pfeil Date: Tue, 10 Dec 2024 10:54:15 -0700 Subject: [PATCH 7/8] =?UTF-8?q?Rephrase=20the=20=E2=80=9Cremove=20`:bug`?= =?UTF-8?q?=E2=80=9D=20transcript=20messages?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/Unison/Codebase/Transcript/Runner.hs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/unison-cli/src/Unison/Codebase/Transcript/Runner.hs b/unison-cli/src/Unison/Codebase/Transcript/Runner.hs index 72de157e82..ee2530a68c 100644 --- a/unison-cli/src/Unison/Codebase/Transcript/Runner.hs +++ b/unison-cli/src/Unison/Codebase/Transcript/Runner.hs @@ -214,11 +214,12 @@ run isTest verbosity dir codebase runtime sbRuntime nRuntime ucmVersion baseURL transcriptFailure out ( Text.pack . Pretty.toPlain terminalWidth $ - "The stanza above previously had an incorrect successful result, but now fails with" - <> "\n" - <> Pretty.border 2 msg - <> "\n" - <> "if this is the expected result, remove `:bug`, otherwise remove `:error`." + Pretty.lines + [ "The stanza above marked with `:error :bug` is now failing with", + Pretty.border 2 msg, + "so you can remove `:bug` and close any appropriate Github issues. If the error message is \ + \different from the expected error message, open a new issue and reference it in this transcript." + ] ) Nothing (_, _) -> pure () @@ -467,7 +468,11 @@ run isTest verbosity dir codebase runtime sbRuntime nRuntime ucmVersion baseURL Nothing (False, True, False) -> do appendFailingStanza - transcriptFailure out "The stanza above is now passing! Please remove `:bug` from it." Nothing + transcriptFailure + out + "The stanza above with `:bug` is now passing! You can remove `:bug` and close any appropriate Github \ + \issues." + Nothing (_, _, _) -> pure () authenticatedHTTPClient <- AuthN.newAuthenticatedHTTPClient tokenProvider ucmVersion From c88d9fd5dc70404359903a61882ecf51225b39e0 Mon Sep 17 00:00:00 2001 From: Greg Pfeil Date: Tue, 10 Dec 2024 15:02:37 -0700 Subject: [PATCH 8/8] Add tests for `:bug` These exposed a few issues with the output formatting, so those are also addressed here. --- .../src/Unison/Codebase/Transcript/Runner.hs | 45 ++++++++++++------- unison-src/transcripts/errors/obsolete-bug.md | 5 +++ .../transcripts/errors/obsolete-bug.output.md | 15 +++++++ .../transcripts/errors/obsolete-error-bug.md | 5 +++ .../errors/obsolete-error-bug.output.md | 19 ++++++++ unison-src/transcripts/idempotent/bug.md | 19 ++++++++ 6 files changed, 93 insertions(+), 15 deletions(-) create mode 100644 unison-src/transcripts/errors/obsolete-bug.md create mode 100644 unison-src/transcripts/errors/obsolete-bug.output.md create mode 100644 unison-src/transcripts/errors/obsolete-error-bug.md create mode 100644 unison-src/transcripts/errors/obsolete-error-bug.output.md create mode 100644 unison-src/transcripts/idempotent/bug.md diff --git a/unison-cli/src/Unison/Codebase/Transcript/Runner.hs b/unison-cli/src/Unison/Codebase/Transcript/Runner.hs index ee2530a68c..97082cfab5 100644 --- a/unison-cli/src/Unison/Codebase/Transcript/Runner.hs +++ b/unison-cli/src/Unison/Codebase/Transcript/Runner.hs @@ -210,18 +210,19 @@ run isTest verbosity dir codebase runtime sbRuntime nRuntime ucmVersion baseURL liftIO $ writeIORef hasErrors True liftIO (liftA2 (,) (readIORef allowErrors) (readIORef expectFailure)) >>= \case (False, False) -> liftIO . dieWithMsg $ Pretty.toPlain terminalWidth msg - (True, True) -> - transcriptFailure - out - ( Text.pack . Pretty.toPlain terminalWidth $ - Pretty.lines - [ "The stanza above marked with `:error :bug` is now failing with", - Pretty.border 2 msg, - "so you can remove `:bug` and close any appropriate Github issues. If the error message is \ - \different from the expected error message, open a new issue and reference it in this transcript." - ] - ) - Nothing + (True, True) -> do + appendFailingStanza + fixedBug out $ + Text.unlines + [ "The stanza above marked with `:error :bug` is now failing with", + "", + "```", + Text.pack $ Pretty.toPlain terminalWidth msg, + "```", + "", + "so you can remove `:bug` and close any appropriate Github issues. If the error message is different \ + \from the expected error message, open a new issue and reference it in this transcript." + ] (_, _) -> pure () apiRequest :: APIRequest -> IO [APIRequest] @@ -467,12 +468,10 @@ run isTest verbosity dir codebase runtime sbRuntime nRuntime ucmVersion baseURL "The transcript was expecting an error in the stanza above, but did not encounter one." Nothing (False, True, False) -> do - appendFailingStanza - transcriptFailure + fixedBug out "The stanza above with `:bug` is now passing! You can remove `:bug` and close any appropriate Github \ \issues." - Nothing (_, _, _) -> pure () authenticatedHTTPClient <- AuthN.newAuthenticatedHTTPClient tokenProvider ucmVersion @@ -528,6 +527,22 @@ transcriptFailure out heading mbody = do <> foldr ((:) . CMarkCodeBlock Nothing "") [] mbody ) +fixedBug :: IORef (Seq Stanza) -> Text -> IO b +fixedBug out body = do + texts <- readIORef out + -- `CMark.commonmarkToNode` returns a @DOCUMENT@, which won’t be rendered inside another document, so we strip the + -- outer `CMark.Node`. + let CMark.Node _ _DOCUMENT bodyNodes = CMark.commonmarkToNode [CMark.optNormalize] body + UnliftIO.throwIO . RunFailure $ + texts + <> Seq.fromList + ( Left + <$> [ CMark.Node Nothing CMark.PARAGRAPH [CMark.Node Nothing (CMark.TEXT "🎉") []], + CMark.Node Nothing (CMark.HEADING 2) [CMark.Node Nothing (CMark.TEXT "You fixed a bug!") []] + ] + <> bodyNodes + ) + data Error = ParseError (P.ParseErrorBundle Text Void) | RunFailure (Seq Stanza) diff --git a/unison-src/transcripts/errors/obsolete-bug.md b/unison-src/transcripts/errors/obsolete-bug.md new file mode 100644 index 0000000000..6f2a9641eb --- /dev/null +++ b/unison-src/transcripts/errors/obsolete-bug.md @@ -0,0 +1,5 @@ +This transcript will error, because we’re claiming that the stanza has a bug, but `help` works as expected. + +``` ucm :bug +scratch/main> help edit +``` diff --git a/unison-src/transcripts/errors/obsolete-bug.output.md b/unison-src/transcripts/errors/obsolete-bug.output.md new file mode 100644 index 0000000000..b88fe47b32 --- /dev/null +++ b/unison-src/transcripts/errors/obsolete-bug.output.md @@ -0,0 +1,15 @@ +This transcript will error, because we’re claiming that the stanza has a bug, but `help` works as expected. + +``` ucm :bug +scratch/main> help edit + + edit + `edit foo` prepends the definition of `foo` to the top of the most recently saved file. + `edit` without arguments invokes a search to select a definition for editing, which requires that `fzf` can be found within your PATH. +``` + +🎉 + +## You fixed a bug\! + +The stanza above with `:bug` is now passing\! You can remove `:bug` and close any appropriate Github issues. diff --git a/unison-src/transcripts/errors/obsolete-error-bug.md b/unison-src/transcripts/errors/obsolete-error-bug.md new file mode 100644 index 0000000000..39b6f667ad --- /dev/null +++ b/unison-src/transcripts/errors/obsolete-error-bug.md @@ -0,0 +1,5 @@ +This transcript will fail, because we’re claiming that the stanza has a bug, but `do.something` errors as expected. + +``` ucm :error :bug +scratch/main> do.something +``` diff --git a/unison-src/transcripts/errors/obsolete-error-bug.output.md b/unison-src/transcripts/errors/obsolete-error-bug.output.md new file mode 100644 index 0000000000..7a3a16789b --- /dev/null +++ b/unison-src/transcripts/errors/obsolete-error-bug.output.md @@ -0,0 +1,19 @@ +This transcript will fail, because we’re claiming that the stanza has a bug, but `do.something` errors as expected. + +``` ucm :error :bug +scratch/main> do.something +``` + +🎉 + +## You fixed a bug\! + +The stanza above marked with `:error :bug` is now failing with + +``` +⚠️ +I don't know how to do.something. Type `help` or `?` to get +help. +``` + +so you can remove `:bug` and close any appropriate Github issues. If the error message is different from the expected error message, open a new issue and reference it in this transcript. diff --git a/unison-src/transcripts/idempotent/bug.md b/unison-src/transcripts/idempotent/bug.md new file mode 100644 index 0000000000..9469b77067 --- /dev/null +++ b/unison-src/transcripts/idempotent/bug.md @@ -0,0 +1,19 @@ +This tests that `:bug` behaves similarly to `:error` when the stanza fails. + +``` ucm :bug +scratch/main> do.something + + ⚠️ + I don't know how to do.something. Type `help` or `?` to get + help. +``` + +And when combined with `:error`, it should expect a successful result. + +``` ucm :error :bug +scratch/main> help edit + + edit + `edit foo` prepends the definition of `foo` to the top of the most recently saved file. + `edit` without arguments invokes a search to select a definition for editing, which requires that `fzf` can be found within your PATH. +```