Skip to content
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

Add support for “known failures” to transcripts #5394

Open
wants to merge 4 commits into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions unison-cli/src/Unison/Codebase/Transcript.hs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

-- | The data model for Unison transcripts.
module Unison.Codebase.Transcript
( ExpectingError,
( Result (..),
ScratchFileName,
Hidden (..),
UcmLine (..),
Expand All @@ -19,7 +19,7 @@ import Unison.Core.Project (ProjectBranchName, ProjectName)
import Unison.Prelude
import Unison.Project (ProjectAndBranch)

type ExpectingError = Bool
data Result = Success | Incorrect | Error | Failure

type ScratchFileName = Text

Expand All @@ -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]
8 changes: 6 additions & 2 deletions unison-cli/src/Unison/Codebase/Transcript/Parser.hs
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,12 @@ hidden =
<|> (HideOutput <$ word ":hide")
<|> pure Shown

expectingError :: P ExpectingError
expectingError = isJust <$> optional (word ":error")
expectingError :: P Result
expectingError =
(Error <$ word ":error")
<|> (Failure <$ word ":failure")
<|> (Incorrect <$ word ":incorrect")
<|> pure Success

untilSpace1 :: P Text
untilSpace1 = P.takeWhile1P Nothing (not . Char.isSpace)
Expand Down
49 changes: 31 additions & 18 deletions unison-cli/src/Unison/Codebase/Transcript/Runner.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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 ..]))
Expand Down Expand Up @@ -265,18 +265,25 @@ 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
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
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
Expand Down Expand Up @@ -365,21 +372,21 @@ 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
Incorrect -> dieWithMsg rendered
_ -> writeIORef hasErrors True

printNumbered :: Output.NumberedOutput -> IO Output.NumberedArgs
printNumbered o = do
let (msg, numberedArgs) = notifyNumbered o
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
Incorrect -> dieWithMsg rendered
_ -> writeIORef hasErrors True
pure numberedArgs

-- Looks at the current stanza and decides if it is contained in the
Expand All @@ -404,10 +411,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

Expand Down
20 changes: 20 additions & 0 deletions unison-src/transcripts-using-base/fix5178.md
Original file line number Diff line number Diff line change
@@ -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
```
45 changes: 45 additions & 0 deletions unison-src/transcripts-using-base/fix5178.output.md
Original file line number Diff line number Diff line change
@@ -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.

```
14 changes: 14 additions & 0 deletions unison-src/transcripts/fix5337.md
Original file line number Diff line number Diff line change
@@ -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`.
32 changes: 32 additions & 0 deletions unison-src/transcripts/fix5337.output.md
Original file line number Diff line number Diff line change
@@ -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`.

Loading