Merge pull request #4413 from unisonweb/fix/update-tests

fix #4405

parser-typechecker/src/Unison/Codebase/MainTerm.hs

@@ -67,12 +67,8 @@ builtinMainWithResultType a res = Type.arrow a (Type.ref a DD.unitRef) io
   where
     io = Type.effect a [Type.builtinIO a, DD.exceptionType a] res
 
--- [Result]
-resultArr :: (Ord v) => a -> Type.Type v a
-resultArr a = Type.app a (Type.ref a Type.listRef) (Type.ref a DD.testResultRef)
-
 builtinResultArr :: (Ord v) => a -> Type.Type v a
-builtinResultArr a = Type.effect a [Type.builtinIO a, DD.exceptionType a] (resultArr a)
+builtinResultArr a = Type.effect a [Type.builtinIO a, DD.exceptionType a] (DD.testResultType a)
 
 -- '{io2.IO} [Result]
 builtinTest :: (Ord v) => a -> Type.Type v a

parser-typechecker/src/Unison/UnisonFile/Type.hs

@@ -21,7 +21,7 @@ data UnisonFile v a = UnisonFileId
     terms :: [(v, a {- ann for whole binding -}, Term v a)],
     watches :: Map WatchKind [(v, a {- ann for whole watch -}, Term v a)]
   }
-  deriving (Show)
+  deriving (Generic, Show)
 
 pattern UnisonFile ::
   Map v (TypeReference, DataDeclaration v a) ->

unison-cli/src/Unison/Codebase/Editor/HandleInput/Update2.hs

@@ -28,6 +28,7 @@ import Text.Pretty.Simple (pShow)
 import U.Codebase.Reference (Reference, ReferenceType)
 import U.Codebase.Reference qualified as Reference
 import U.Codebase.Sqlite.Operations qualified as Ops
+import Unison.Builtin.Decls qualified as Decls
 import Unison.Cli.Monad (Cli)
 import Unison.Cli.Monad qualified as Cli
 import Unison.Cli.MonadUtils qualified as Cli
@@ -79,6 +80,7 @@ import Unison.Syntax.Name qualified as Name
 import Unison.Syntax.Parser qualified as Parser
 import Unison.Term (Term)
 import Unison.Type (Type)
+import Unison.Typechecker qualified as Typechecker
 import Unison.UnisonFile qualified as UF
 import Unison.UnisonFile.Names qualified as UF
 import Unison.UnisonFile.Type (TypecheckedUnisonFile, UnisonFile)
@@ -88,6 +90,7 @@ import Unison.Util.Pretty (Pretty)
 import Unison.Util.Pretty qualified as Pretty
 import Unison.Util.Relation qualified as Relation
 import Unison.Var (Var)
+import Unison.WatchKind qualified as WK
 
 handleUpdate2 :: Cli ()
 handleUpdate2 = do
@@ -287,16 +290,23 @@ addDefinitionsToUnisonFile abort c names ctorNames dependents initialUnisonFile
       pure $ foldl' addTermElement uf (zip termComponent [0 ..])
       where
         addTermElement :: UnisonFile Symbol Ann -> ((Term Symbol Ann, Type Symbol Ann), Reference.Pos) -> UnisonFile Symbol Ann
-        addTermElement uf ((tm, _tp), i) = do
+        addTermElement uf ((tm, tp), i) = do
           let r :: Referent = Referent.Ref $ Reference.Derived h i
               termNames = Relation.lookupRan r names.terms
-          foldl' (addDefinition tm) uf termNames
-        addDefinition :: Term Symbol Ann -> UnisonFile Symbol Ann -> Name -> UnisonFile Symbol Ann
-        addDefinition tm uf (Name.toVar -> v) =
+          foldl' (addDefinition tm tp) uf termNames
+        addDefinition :: Term Symbol Ann -> Type Symbol Ann -> UnisonFile Symbol Ann -> Name -> UnisonFile Symbol Ann
+        addDefinition tm tp uf (Name.toVar -> v) =
           if Set.member v termNames
             then uf
-            else uf {UF.terms = (v, Ann.External, tm) : uf.terms}
-        termNames = Set.fromList [v | (v, _, _) <- uf.terms]
+            else
+              let prependTerm to = (v, Ann.External, tm) : to
+               in if isTest tp
+                    then uf & #watches . Lens.at WK.TestWatch . Lens.non [] Lens.%~ prependTerm
+                    else uf & #terms Lens.%~ prependTerm
+        termNames =
+          Set.fromList [v | (v, _, _) <- uf.terms]
+            <> foldMap (\x -> Set.fromList [v | (v, _, _) <- x]) uf.watches
+    isTest = Typechecker.isEqual (Decls.testResultType mempty)
 
     -- given a dependent hash, include that component in the scratch file
     -- todo: wundefined: cut off constructor name prefixes

unison-cli/src/Unison/CommandLine/OutputMessages.hs

@@ -2525,6 +2525,7 @@ prettyUnisonFile ppe uf@(UF.UnisonFileId datas effects terms watches) =
             | Var.UnnamedWatch _ _ <- Var.typeOf v ->
                 "> " <> P.indentNAfterNewline 2 (TermPrinter.pretty sppe tm)
           WK.RegularWatch -> "> " <> pb (hqv v) tm
+          WK.TestWatch -> "test> " <> st (TermPrinter.prettyBindingWithoutTypeSignature sppe (hqv v) tm)
           w -> P.string w <> "> " <> pb (hqv v) tm
     st = P.syntaxToColor
     sppe = PPED.suffixifiedPPE ppe'

unison-src/transcripts/add-test-watch-roundtrip.md

@@ -0,0 +1,15 @@
+```ucm:hide
+.> builtins.mergeio
+```
+
+```unison:hide
+test> foo : [Test.Result]
+foo = []
+```
+
+Apparently when we add a test watch, we add a type annotation to it, even if it already has one. We don't want this to happen though!
+
+```ucm
+.> add
+.> view foo
+```

unison-src/transcripts/add-test-watch-roundtrip.output.md

@@ -0,0 +1,21 @@
+```unison
+test> foo : [Test.Result]
+foo = []
+```
+
+Apparently when we add a test watch, we add a type annotation to it, even if it already has one. We don't want this to happen though!
+
+```ucm
+.> add
+
+  ⍟ I've added these definitions:
+  
+    foo : [Result]
+
+.> view foo
+
+  foo : [Result]
+  foo : [Result]
+  foo = []
+
+```

unison-src/transcripts/update-test-to-non-test.md

@@ -0,0 +1,25 @@
+```ucm
+.> builtins.mergeio
+```
+
+```unison
+test> foo = []
+```
+
+After adding the test `foo`, we expect `view` to render it like a test. (Bug: It doesn't.)
+
+```ucm
+.> add
+.> view foo
+```
+
+```unison
+foo = 1
+```
+
+After updating `foo` to not be a test, we expect `view` to not render it like a test.
+
+```ucm
+.> update
+.> view foo
+```

unison-src/transcripts/update-test-to-non-test.output.md

@@ -0,0 +1,78 @@
+```ucm
+.> builtins.mergeio
+
+  Done.
+
+```
+```unison
+test> foo = []
+```
+
+```ucm
+
+  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 : [Result]
+  
+  Now evaluating any watch expressions (lines starting with
+  `>`)... Ctrl+C cancels.
+
+    1 | test> foo = []
+    
+
+```
+After adding the test `foo`, we expect `view` to render it like a test. (Bug: It doesn't.)
+
+```ucm
+.> add
+
+  ⍟ I've added these definitions:
+  
+    foo : [Result]
+
+.> view foo
+
+  foo : [Result]
+  foo = []
+
+```
+```unison
+foo = 1
+```
+
+```ucm
+
+  I found and typechecked these definitions in scratch.u. If you
+  do an `add` or `update`, here's how your codebase would
+  change:
+  
+    ⍟ These names already exist. You can `update` them to your
+      new definition:
+    
+      foo : Nat
+
+```
+After updating `foo` to not be a test, we expect `view` to not render it like a test.
+
+```ucm
+.> update
+
+  Okay, I'm searching the branch for code that needs to be
+  updated...
+
+  That's done. Now I'm making sure everything typechecks...
+
+  Everything typechecks, so I'm saving the results...
+
+  Done.
+
+.> view foo
+
+  foo : Nat
+  foo = 1
+
+```

unison-src/transcripts/update-test-watch-roundtrip.md

@@ -0,0 +1,28 @@
+
+```ucm:hide
+.> builtins.mergeio
+```
+
+Given a test that depends on another definition,
+
+```unison:hide
+foo n = n + 1
+
+test> mynamespace.foo.test =
+  n = 2
+  if (foo n) == 2 then [ Ok "passed" ] else [ Fail "wat" ]
+```
+
+```ucm
+.> add
+```
+
+if we change the type of the dependency, the test should show in the scratch file as a test watch.
+
+```unison
+foo n = "hello, world!"
+```
+
+```ucm:error
+.> update
+```

unison-src/transcripts/update-test-watch-roundtrip.output.md

@@ -0,0 +1,57 @@
+
+Given a test that depends on another definition,
+
+```unison
+foo n = n + 1
+
+test> mynamespace.foo.test =
+  n = 2
+  if (foo n) == 2 then [ Ok "passed" ] else [ Fail "wat" ]
+```
+
+```ucm
+.> add
+
+  ⍟ I've added these definitions:
+  
+    foo                  : Nat -> Nat
+    mynamespace.foo.test : [Result]
+
+```
+if we change the type of the dependency, the test should show in the scratch file as a test watch.
+
+```unison
+foo n = "hello, world!"
+```
+
+```ucm
+
+  I found and typechecked these definitions in scratch.u. If you
+  do an `add` or `update`, here's how your codebase would
+  change:
+  
+    ⍟ These names already exist. You can `update` them to your
+      new definition:
+    
+      foo : n -> Text
+
+```
+```ucm
+.> update
+
+  Okay, I'm searching the branch for code that needs to be
+  updated...
+
+  That's done. Now I'm making sure everything typechecks...
+
+  test> mynamespace.foo.test =
+    n = 2
+    if foo n == 2 then [Ok "passed"] else [Fail "wat"]
+  
+  foo n = "hello, world!"
+
+  Typechecking failed. I've updated your scratch file with the
+  definitions that need fixing. Once the file is compiling, try
+  `update` again.
+
+```