gitutils: add diffStrings, diffFiles, and use it in testament to compare expected vs gotten (#17892)

* gitutils: add diffStrings, diffFiles, and use it in testament to compare expected vs gotten
* refactor with createTempDir
* cleanup
* refacotr
* PRTEMP fake test spec changes to show effect of diffStrings
* add runnableExamples for experimental/diff + cross-reference with gitutils
* Revert "PRTEMP fake test spec changes to show effect of diffStrings"

This reverts commit 57dc8d642dce6c1127c98b7cbc9edbfe747d4047.

lib/experimental/diff.nim

@@ -10,28 +10,31 @@
 ## This module implements an algorithm to compute the
 ## `diff`:idx: between two sequences of lines.
 ##
-## A basic example of `diffInt` on 2 arrays of integers:
-##
-## .. code:: Nim
-##
-##   import experimental/diff
-##   echo diffInt([0, 1, 2, 3, 4, 5, 6, 7, 8], [-1, 1, 2, 3, 4, 5, 666, 7, 42])
-##
-## Another short example of `diffText` to diff strings:
-##
-## .. code:: Nim
-##
-##   import experimental/diff
-##   # 2 samples of text for testing (from "The Call of Cthulhu" by Lovecraft)
-##   let txt0 = """I have looked upon all the universe has to hold of horror,
-##   even skies of spring and flowers of summer must ever be poison to me."""
-##   let txt1 = """I have looked upon all your code has to hold of bugs,
-##   even skies of spring and flowers of summer must ever be poison to me."""
-##
-##   echo diffText(txt0, txt1)
-##
 ## - To learn more see `Diff on Wikipedia. <http://wikipedia.org/wiki/Diff>`_
 
+runnableExamples:
+  assert diffInt(
+    [0, 1, 2, 3, 4, 5, 6, 7, 8],
+    [-1, 1, 2, 3, 4, 5, 666, 7, 42]) ==
+    @[Item(startA: 0, startB: 0, deletedA: 1, insertedB: 1),
+      Item(startA: 6, startB: 6, deletedA: 1, insertedB: 1),
+      Item(startA: 8, startB: 8, deletedA: 1, insertedB: 1)]
+
+runnableExamples:
+  # 2 samples of text (from "The Call of Cthulhu" by Lovecraft)
+  let txt0 = """
+abc
+def ghi
+jkl2"""
+  let txt1 = """
+bacx
+abc
+def ghi
+jkl"""
+  assert diffText(txt0, txt1) ==
+    @[Item(startA: 0, startB: 0, deletedA: 0, insertedB: 1),
+      Item(startA: 2, startB: 3, deletedA: 1, insertedB: 1)]
+
 # code owner: Arne Döring
 #
 # This is based on C# code written by Matthias Hertel, http://www.mathertel.de
@@ -309,7 +312,7 @@ proc diffText*(textA, textB: string): seq[Item] =
   ## `textB` B-version of the text (usually the new one)
   ##
   ## Returns a seq of Items that describe the differences.
-
+  # See also `gitutils.diffStrings`.
   # prepare the input-text and convert to comparable numbers.
   var h = initTable[string, int]()  # TextA.len + TextB.len  <- probably wrong initial size
   # The A-Version of the data (original data) to be compared.

lib/std/private/gitutils.nim

@@ -4,7 +4,7 @@ internal API for now, API subject to change
 
 # xxx move other git utilities here; candidate for stdlib.
 
-import std/[os, osproc, strutils]
+import std/[os, osproc, strutils, tempfiles]
 
 const commitHead* = "HEAD"
 
@@ -38,3 +38,41 @@ proc isGitRepo*(dir: string): bool =
   # usually a series of ../), so we know that it's safe to unconditionally
   # remove trailing whitespaces from the result.
   result = status == 0 and output.strip() == ""
+
+proc diffFiles*(path1, path2: string): tuple[output: string, same: bool] =
+  ## Returns a human readable diff of files `path1`, `path2`, the exact form of
+  ## which is implementation defined.
+  # This could be customized, e.g. non-git diff with `diff -uNdr`, or with
+  # git diff options (e.g. --color-moved, --word-diff).
+  # in general, `git diff` has more options than `diff`.
+  var status = 0
+  (result.output, status) = execCmdEx("git diff --no-index $1 $2" % [path1.quoteShell, path2.quoteShell])
+  doAssert (status == 0) or (status == 1)
+  result.same = status == 0
+
+proc diffStrings*(a, b: string): tuple[output: string, same: bool] =
+  ## Returns a human readable diff of `a`, `b`, the exact form of which is
+  ## implementation defined.
+  ## See also `experimental.diff`.
+  runnableExamples:
+    let a = "ok1\nok2\nok3\n"
+    let b = "ok1\nok2 alt\nok3\nok4\n"
+    let (c, same) = diffStrings(a, b)
+    doAssert not same
+    let (c2, same2) = diffStrings(a, a)
+    doAssert same2
+  runnableExamples("-r:off"):
+    let a = "ok1\nok2\nok3\n"
+    let b = "ok1\nok2 alt\nok3\nok4\n"
+    echo diffStrings(a, b).output
+
+  template tmpFileImpl(prefix, str): auto =
+    let path = genTempPath(prefix, "")
+    writeFile(path, str)
+    path
+  let patha = tmpFileImpl("diffStrings_a_", a)
+  let pathb = tmpFileImpl("diffStrings_b_", b)
+  defer:
+    removeFile(patha)
+    removeFile(pathb)
+  result = diffFiles(patha, pathb)

lib/std/tempfiles.nim

@@ -90,25 +90,33 @@ template randomPathName(length: Natural): string =
     res[i] = state.sample(letters)
   res
 
+proc getTempDirImpl(dir: string): string {.inline.} =
+  result = dir
+  if result.len == 0:
+    result = getTempDir()
+
+proc genTempPath*(prefix, suffix: string, dir = ""): string =
+  ## Generates a path name in `dir`.
+  ##
+  ## If `dir` is empty, (`getTempDir <os.html#getTempDir>`_) will be used.
+  ## The path begins with `prefix` and ends with `suffix`.
+  let dir = getTempDirImpl(dir)
+  result = dir / (prefix & randomPathName(nimTempPathLength) & suffix)
+
 proc createTempFile*(prefix, suffix: string, dir = ""): tuple[fd: File, path: string] =
-  ## `createTempFile` creates a new temporary file in the directory `dir`.
+  ## Creates a new temporary file in the directory `dir`.
   ## 
-  ## If `dir` is the empty string, the default directory for temporary files
+  ## This generates a path name using `genTempPath(prefix, suffix, dir)` and
-  ## (`getTempDir <os.html#getTempDir>`_) will be used.
+  ## returns a file handle to an open file and the path of that file, possibly after
-  ## The temporary file name begins with `prefix` and ends with `suffix`.
+  ## retrying to ensure it doesn't already exist.
-  ## `createTempFile` returns a file handle to an open file and the path of that file.
   ## 
   ## If failing to create a temporary file, `IOError` will be raised.
   ##
   ## .. note:: It is the caller's responsibility to remove the file when no longer needed.
-  var dir = dir
+  let dir = getTempDirImpl(dir)
-  if dir.len == 0:
-    dir = getTempDir()
-
   createDir(dir)
-
   for i in 0 ..< maxRetry:
-    result.path = dir / (prefix & randomPathName(nimTempPathLength) & suffix)
+    result.path = genTempPath(prefix, suffix, dir)
     try:
       result.fd = safeOpen(result.path)
     except OSError:
@@ -118,25 +126,19 @@ proc createTempFile*(prefix, suffix: string, dir = ""): tuple[fd: File, path: st
   raise newException(IOError, "Failed to create a temporary file under directory " & dir)
 
 proc createTempDir*(prefix, suffix: string, dir = ""): string =
-  ## `createTempDir` creates a new temporary directory in the directory `dir`.
+  ## Creates a new temporary directory in the directory `dir`.
   ##
-  ## If `dir` is the empty string, the default directory for temporary files
+  ## This generates a dir name using `genTempPath(prefix, suffix, dir)`, creates
-  ## (`getTempDir <os.html#getTempDir>`_) will be used.
+  ## the directory and returns it, possibly after retrying to ensure it doesn't
-  ## The temporary directory name begins with `prefix` and ends with `suffix`.
+  ## already exist.
-  ## `createTempDir` returns the path of that temporary firectory.
   ##
   ## If failing to create a temporary directory, `IOError` will be raised.
   ##
   ## .. note:: It is the caller's responsibility to remove the directory when no longer needed.
-  ##
+  let dir = getTempDirImpl(dir)
-  var dir = dir
-  if dir.len == 0:
-    dir = getTempDir()
-
   createDir(dir)
-
   for i in 0 ..< maxRetry:
-    result = dir / (prefix & randomPathName(nimTempPathLength) & suffix)
+    result = genTempPath(prefix, suffix, dir)
     try:
       if not existsOrCreateDir(result):
         return

nimdoc/rsttester.nim

@@ -1,4 +1,5 @@
 import os, strutils
+from std/private/gitutils import diffFiles
 
 const
   baseDir = "nimdoc/rst2html"
@@ -19,7 +20,7 @@ proc testRst2Html(fixup = false) =
     exec("$1 rst2html $2" % [nimExe, sourceFile])
     let producedHtml = expectedHtml.replace('\\', '/').replace("/expected/", "/source/htmldocs/")
     if readFile(expectedHtml) != readFile(producedHtml):
-      discard execShellCmd("diff -uNdr " & expectedHtml & " " & producedHtml)
+      echo diffFiles(expectedHtml, producedHtml).output
       inc failures
       if fixup:
         copyFile(producedHtml, expectedHtml)

nimdoc/tester.nim

@@ -3,6 +3,7 @@
 # to change expected results (after carefully verifying everything), use -d:fixup
 
 import strutils, os
+from std/private/gitutils import diffFiles
 
 var
   failures = 0
@@ -40,7 +41,7 @@ proc testNimDoc(prjDir, docsDir: string; switches: NimSwitches; fixup = false) =
       inc failures
     elif readFile(expected) != readFile(produced):
       echo "FAILURE: files differ: ", produced
-      discard execShellCmd("diff -uNdr " & expected & " " & produced)
+      echo diffFiles(expected, produced).output
       inc failures
       if fixup:
         copyFile(produced, expected)

nimpretty/tester.nim

@@ -1,6 +1,7 @@
 # Small program that runs the test cases
 
 import strutils, os, sequtils
+from std/private/gitutils import diffFiles
 
 const
   dir = "nimpretty/tests"
@@ -26,7 +27,7 @@ proc test(infile, ext: string) =
   let produced = dir / nimFile.changeFileExt(ext)
   if readFile(expected) != readFile(produced):
     echo "FAILURE: files differ: ", nimFile
-    discard execShellCmd("diff -uNdr " & expected & " " & produced)
+    echo diffFiles(expected, produced).output
     failures += 1
   else:
     echo "SUCCESS: files identical: ", nimFile
@@ -43,7 +44,7 @@ proc testTogether(infiles: seq[string]) =
     let produced = dir / "outputdir" / infile
     if readFile(expected) != readFile(produced):
       echo "FAILURE: files differ: ", nimFile
-      discard execShellCmd("diff -uNdr " & expected & " " & produced)
+      echo diffFiles(expected, produced).output
       failures += 1
     else:
       echo "SUCCESS: files identical: ", nimFile

testament/categories.nim

@@ -665,8 +665,8 @@ proc runJoinedTest(r: var TResults, cat: Category, testsDir: string, options: st
 
   if buf != outputExpected:
     writeFile(outputExceptedFile, outputExpected)
-    discard execShellCmd("diff -uNdr $1 $2" % [outputExceptedFile, outputGottenFile])
+    echo diffFiles(outputGottenFile, outputExceptedFile).output
-    echo failString & "megatest output different!"
+    echo failString & "megatest output different, see $1 vs $2" % [outputGottenFile, outputExceptedFile]
     # outputGottenFile, outputExceptedFile not removed on purpose for debugging.
     quit 1
   else:

testament/testament.nim

@@ -17,6 +17,7 @@ from std/sugar import dup
 import compiler/nodejs
 import lib/stdtest/testutils
 from lib/stdtest/specialpaths import splitTestFile
+from std/private/gitutils import diffStrings
 
 proc trimUnitSep(x: var string) =
   let L = x.len
@@ -307,7 +308,7 @@ proc addResult(r: var TResults, test: TTest, target: TTarget,
       maybeStyledEcho styleBright, expected, "\n"
       maybeStyledEcho fgYellow, "Gotten:"
       maybeStyledEcho styleBright, given, "\n"
-
+      echo diffStrings(expected, given).output
 
   if backendLogging and (isAppVeyor or isAzure):
     let (outcome, msg) =