Merge pull request #298 from LoopPerfect/bugfix/275

* Improves TOML formatting; adds tests
LoopPerfect · Jan 30, 2019 · 2327f4f · 2327f4f
2 parents 8dd55cc + 3f7c43b
commit 2327f4f
Show file tree

Hide file tree

Showing 4 changed files with 112 additions and 51 deletions.
diff --git a/buckaroo-tests/Lock.fs b/buckaroo-tests/Lock.fs
@@ -1,6 +1,7 @@
 module Buckaroo.Tests.Lock
 
 open Xunit
+open FSharpx
 open Buckaroo
 
 [<Fact>]
@@ -18,6 +19,14 @@ let ``Lock.parse works correctly 1`` () =
     Packages = Map.empty;
   }
 
+  // 3 new-lines indicates poor formatting
+  Assert.True (
+    expected
+    |> Lock.toToml
+    |> String.contains "\n\n\n"
+    |> not
+  )
+
   Assert.Equal(Result.Ok expected, actual)
 
 [<Fact>]
@@ -73,6 +82,14 @@ let ``Lock.parse works correctly 2`` () =
         };
   }
 
+  // 3 new-lines indicates poor formatting
+  Assert.True (
+    expected
+    |> Lock.toToml
+    |> String.contains "\n\n\n"
+    |> not
+  )
+
   Assert.Equal(Result.Ok expected, actual)
 
 [<Fact>]
@@ -150,4 +167,12 @@ let ``Lock.parse works correctly 3`` () =
         };
   }
 
+  // 3 new-lines indicates poor formatting
+  Assert.True (
+    expected
+    |> Lock.toToml
+    |> String.contains "\n\n\n"
+    |> not
+  )
+
   Assert.Equal(Result.Ok expected, actual)
diff --git a/buckaroo-tests/Manifest.fs b/buckaroo-tests/Manifest.fs
@@ -2,10 +2,12 @@ module Buckaroo.Tests.Manifest
 
 open System
 open Xunit
+open FSharpx
 
 open Buckaroo
 open Buckaroo.Tests
 open Buckaroo
+open Buckaroo.Tests
 
 [<Fact>]
 let ``Manifest.parse works correctly`` () =
@@ -63,22 +65,47 @@ let ``Manifest.parse works correctly`` () =
             Manifest.zero with
               Dependencies = set [ c ];
               Locations = Map.ofSeq [
-                (
-                  lmnqrs,
-                  locationC
-                )
+                (lmnqrs, locationC)
               ]
           }
       );
     ]
 
   for (input, expected) in cases do
     let actual = Manifest.parse input
+
     Assert.Equal(expected, actual)
 
 
 [<Fact>]
-let ``Manifest.toToml roundtrip`` () =
+let ``Manifest.toToml roundtrip 1`` () =
+  let expected : Manifest = {
+    Manifest.zero with
+      Targets = Set [
+        {Folders=["foo"; "bar"]; Name = "xxx"}
+        {Folders=["foo"; "bar"]; Name = "yyy"}
+      ]
+      Dependencies = Set [{
+        Targets = Some ([{Folders=["foo"; "bar"]; Name = "xxx"}])
+        Constraint = All [Constraint.Exactly (Version.SemVer SemVer.zero)]
+        Package = PackageIdentifier.GitHub { Owner = "abc"; Project = "def" }
+      }]
+  }
+
+  let actual =  expected |> Manifest.toToml |> Manifest.parse
+
+  // 3 new-lines indicates poor formatting
+  Assert.True (
+    expected
+    |> Manifest.toToml
+    |> String.contains "\n\n\n"
+    |> not
+  )
+
+  Assert.Equal (Result.Ok expected, actual)
+
+[<Fact>]
+let ``Manifest.toToml roundtrip 2`` () =
   let expected : Manifest = {
     Targets = Set [
       {Folders=["foo"; "bar"]; Name = "xxx"}
@@ -87,7 +114,7 @@ let ``Manifest.toToml roundtrip`` () =
     Tags = Set ["c++"; "java"; "ml"]
     Locations = Map.ofSeq [
       ({Owner = "test"; Project = "test1"}, PackageSource.Http (Map.ofSeq [
-        (Version.SemVer SemVer.zero, {
+        (Version.SemVer { SemVer.zero with Major = 2 }, {
           Url = "https://test.com"
           StripPrefix = Some "prefix"
           Type = Some ArchiveType.Zip
@@ -108,4 +135,12 @@ let ``Manifest.toToml roundtrip`` () =
 
   let actual =  expected |> Manifest.toToml |> Manifest.parse
 
-  Assert.Equal (Result.Ok expected, actual)
+  // 3 new-lines indicates poor formatting
+  Assert.True (
+    expected
+    |> Manifest.toToml
+    |> String.contains "\n\n\n"
+    |> not
+  )
+
+  Assert.Equal (Result.Ok expected, actual)
diff --git a/buckaroo/Manifest.fs b/buckaroo/Manifest.fs
@@ -337,57 +337,59 @@ module Manifest =
     (
       match x.Tags |> Seq.exists (fun _ -> true) with
       | true ->
-        "tags = [ " + (
-          x.Tags
-          |> Seq.distinct
-          |> Seq.sort
-          |> Seq.map (fun x -> "\"" + x + "\"")
-          |> String.concat ", "
-        ) + " ]\n\n"
+          "tags = [ " + (
+            x.Tags
+            |> Seq.distinct
+            |> Seq.sort
+            |> Seq.map (fun x -> "\"" + x + "\"")
+            |> String.concat ", "
+          ) + " ]\n\n"
       | false -> ""
     ) +
     (
       match x.Targets |> Seq.exists (fun _ -> true) with
       | true ->
-        "targets = [ " + (
-          x.Targets
-          |> Seq.distinct
-          |> Seq.sort
-          |> Seq.map (fun x -> "\"" + (Target.show x) + "\"")
-          |> String.concat ", "
-        ) + " ]\n\n"
+          "targets = [ " + (
+            x.Targets
+            |> Seq.distinct
+            |> Seq.sort
+            |> Seq.map (fun x -> "\"" + (Target.show x) + "\"")
+            |> String.concat ", "
+          ) + " ]\n\n"
       | false -> ""
     ) +
     (
       x.Locations
       |> Map.toSeq
       |> Seq.sortBy fst
       |> Seq.map (fun (package, source) ->
-        match source with
+        (
+          match source with
           | PackageSource.Git git ->
-            "[[location]]\n" +
-            "package = \"" + PackageIdentifier.show (PackageIdentifier.Adhoc package) + "\"\n" +
-            "git = \"" + git.Uri + "\"\n"
+              "[[location]]\n" +
+              "package = \"" + PackageIdentifier.show (PackageIdentifier.Adhoc package) + "\"\n" +
+              "git = \"" + git.Uri + "\"\n"
           | PackageSource.Http http ->
-            http
-              |> Map.toSeq
-              |> Seq.sortBy fst
-              |> Seq.map (fun (version, h) ->
-                "[[location]]\n" +
-                "package = \"" + PackageIdentifier.show (PackageIdentifier.Adhoc package) + "\"\n" +
-                "version = \"" + (Version.show version) + "\"\n" +
-                "url = \"" + h.Url + "\"\n" +
-                (h.StripPrefix
-                  |> Option.map(fun p -> "strip_prefix = \"" + p + "\"\n")
-                  |> Option.defaultValue("")) +
-                (h.Type
-                  |> Option.map(fun t -> "type = \"" + t.ToString() + "\"\n")
-                  |> Option.defaultValue(""))
-              )
-              |> String.concat "\n"
+              http
+                |> Map.toSeq
+                |> Seq.sortBy fst
+                |> Seq.map (fun (version, h) ->
+                  "[[location]]\n" +
+                  "package = \"" + PackageIdentifier.show (PackageIdentifier.Adhoc package) + "\"\n" +
+                  "version = \"" + (Version.show version) + "\"\n" +
+                  "url = \"" + h.Url + "\"\n" +
+                  (h.StripPrefix
+                    |> Option.map(fun p -> "strip_prefix = \"" + p + "\"\n")
+                    |> Option.defaultValue("")) +
+                  (h.Type
+                    |> Option.map(fun t -> "type = \"" + t.ToString() + "\"\n")
+                    |> Option.defaultValue(""))
+                )
+                |> String.concat "\n"
+        ) + "\n"
       )
-      |> String.concat "\n"
-    ) + "\n" +
+      |> String.concat ""
+    ) +
     (
       Seq.append
         (x.Dependencies
@@ -398,19 +400,18 @@ module Manifest =
         "[[dependency]]\n" +
         "package = \"" + PackageIdentifier.show x.Package + "\"\n" +
         "version = \"" + Constraint.show x.Constraint + "\"\n" +
-        (if isPrivate
-         then "private = true\n"
-         else "") +
+        (if isPrivate then "private = true\n" else "") +
         (
           match x.Targets with
           | Some ts ->
             "targets = [ " +
             (ts |> Seq.map (fun t -> "\"" + Target.show t + "\"") |> String.concat ", ") +
             " ]\n"
           | None -> ""
-        )
+        ) +
+        "\n"
       )
-      |> String.concat "\n"
+      |> String.concat ""
     )
 
   let hash manifest =

diff --git a/buckaroo/Toml.fs b/buckaroo/Toml.fs
@@ -3,9 +3,9 @@ module Buckaroo.Toml
 open System
 
 type TomlError =
-| CouldNotParse of Exception
-| KeyNotFound of string
-| UnexpectedType of string
+  | CouldNotParse of Exception
+  | KeyNotFound of string
+  | UnexpectedType of string
 
 module TomlError =
   let show (x : TomlError) =