Hide section about NixPkgs review when it was skipped

We're no longer running this on `staging` PRs, and users doing single-package
updates have the option to omit `nixpkgs-review`, so we should only include the
PR comment section if it was actually run.

This continues to enhance the automated tests for the PR comment formatting,
so you can see what the diff looks like just by looking at the test suite!

.gitignore

@@ -1,8 +1,9 @@
+.ghc*
 /github_token.txt
+/nixpkgs-update.nix
 /packages-to-update.txt
 /result
-dist/
 dist-newstyle/
-/nixpkgs-update.nix
+dist/
-.ghc*
 store-path-pre-build
+test_data/actual*

src/Update.hs

@@ -394,6 +394,19 @@ prMessage updateEnv isBroken metaDescription metaHomepage rewriteMsgs releaseUrl
         if compareUrl == T.empty
           then ""
           else "\n- [Compare changes on GitHub](" <> compareUrl <> ")\n\n"
+      nixpkgsReviewSection =
+        if nixpkgsReviewMsg == T.empty
+          then "NixPkgs review skipped"
+          else [interpolate|
+            We have automatically built all packages that will get rebuilt due to
+            this change.
+
+            This gives evidence on whether the upgrade will break dependent packages.
+            Note sometimes packages show up as _failed to build_ independent of the
+            change, simply because they are already broken on the target branch.
+
+            $nixpkgsReviewMsg
+            |]
       pat link = [interpolate|This update was made based on information from $link.|]
       sourceLinkInfo = maybe "" pat $ sourceURL updateEnv
    in [interpolate|
@@ -459,14 +472,9 @@ prMessage updateEnv isBroken metaDescription metaHomepage rewriteMsgs releaseUrl
 
        $cveRep
 
-       # Pre-merge build results
+       ### Pre-merge build results
 
-       We have automatically built all packages that will get rebuilt due to this change.
+       $nixpkgsReviewSection
-
-       This gives evidence on whether the upgrade will break dependent packages.
-       Note sometimes packages show up as _failed to build_ independent of the change, simply because they are already broken on the target branch.
-
-       $nixpkgsReviewMsg
 
        ---
 

test/UpdateSpec.hs

@@ -10,11 +10,14 @@ import qualified Utils
 main :: IO ()
 main = hspec spec
 
+-- Tests here will write their actual results to an actual*.md file next to the
+-- expected one. If you are updating the PR description, run the test suite and
+-- copy all the actuals over to expected. This can be viewed with `git diff`
+-- then to ensure your changes look as expected.
 spec :: Spec
 spec = do
   describe "PR message" do
-    it "matches a simple mock example" do
+    -- Common mock options
-      expected <- T.readFile "test_data/expected_pr_description_1.md"
     let options = Utils.Options False False "" False False False False
     let updateEnv = Utils.UpdateEnv "foobar" "1.0" "1.1" (Just "https://update-site.com") options
     let isBroken = False
@@ -32,5 +35,16 @@ spec = do
     let cveRep = ""
     let cachixTestInstructions = ""
     let nixpkgsReviewMsg = "nixpkgs-review comment body"
+
+    it "matches a simple mock example" do
+      expected <- T.readFile "test_data/expected_pr_description_1.md"
       let actual = Update.prMessage updateEnv isBroken metaDescription metaHomepage rewriteMsgs releaseUrl compareUrl resultCheckReport commitHash attrPath maintainersCc resultPath opReport cveRep cachixTestInstructions nixpkgsReviewMsg
+      T.writeFile "test_data/actual_pr_description_1.md" actual
+      actual `shouldBe` expected
+
+    it "does not include NixPkgs review section when no review was done" do
+      expected <- T.readFile "test_data/expected_pr_description_2.md"
+      let nixpkgsReviewMsg' = ""
+      let actual = Update.prMessage updateEnv isBroken metaDescription metaHomepage rewriteMsgs releaseUrl compareUrl resultCheckReport commitHash attrPath maintainersCc resultPath opReport cveRep cachixTestInstructions nixpkgsReviewMsg'
+      T.writeFile "test_data/actual_pr_description_2.md" actual
       actual `shouldBe` expected

test_data/expected_pr_description_1.md

@@ -69,12 +69,14 @@ ls -la /nix/store/some-hash-path/bin
 
 
 
-# Pre-merge build results
+### Pre-merge build results
 
-We have automatically built all packages that will get rebuilt due to this change.
+We have automatically built all packages that will get rebuilt due to
+this change.
 
 This gives evidence on whether the upgrade will break dependent packages.
-Note sometimes packages show up as _failed to build_ independent of the change, simply because they are already broken on the target branch.
+Note sometimes packages show up as _failed to build_ independent of the
+change, simply because they are already broken on the target branch.
 
 nixpkgs-review comment body
 

test_data/expected_pr_description_2.md

@@ -0,0 +1,82 @@
+Semi-automatic update generated by [nixpkgs-update](https://github.com/ryantm/nixpkgs-update) tools. This update was made based on information from https://update-site.com.
+
+
+
+meta.description for foobar is: "Foobar package description"
+
+meta.homepage for foobar is: "https://foobar-homepage.com"
+
+###### Updates performed
+- Version Update
+- Other Update
+
+###### To inspect upstream changes
+
+
+- [Release on GitHub](https://github.com/foobar/releases)
+
+- [Compare changes on GitHub](https://github.com/foobar/compare)
+
+
+###### Impact
+
+<details>
+<summary>
+<b>Checks done</b> (click to expand)
+</summary>
+
+---
+
+- built on NixOS
+- Some other check
+
+---
+
+</details>
+<details>
+<summary>
+<b>Rebuild report</b> (if merged into master) (click to expand)
+</summary>
+
+```
+123 total rebuild path(s)
+```
+
+</details>
+
+<details>
+<summary>
+<b>Instructions to test this update</b> (click to expand)
+</summary>
+
+---
+
+
+```
+nix-build -A foobar https://github.com/r-ryantm/nixpkgs/archive/af39cf77a0d42a4f6771043ec54221ed.tar.gz
+```
+
+After you've downloaded or built it, look at the files and if there are any, run the binaries:
+```
+ls -la /nix/store/some-hash-path
+ls -la /nix/store/some-hash-path/bin
+```
+
+---
+
+</details>
+<br/>
+
+
+
+### Pre-merge build results
+
+NixPkgs review skipped
+
+---
+
+###### Maintainer pings
+
+
+
+cc @maintainer1 for testing.