From 68554920e55e04a22c11fc6ab2780290abba8ed0 Mon Sep 17 00:00:00 2001
From: Jonathan Bernard <jonathan@jdbernard.com>
Date: Sun, 16 Apr 2023 03:34:14 -0500
Subject: [PATCH] Fix bug in parsing TEL content. Rework unit tests.

- newVC3_Tel was not assigning the value provided to the constructed
  object.
- Private unit tests were run every time the code was compiled due to
  how the unittest library works. These now only run as part of the unit
  tests with `nimble test`.
---
 src/vcard/private/lexer.nim | 155 +++++++++++++++---------------
 src/vcard/vcard3.nim        | 181 ++++++++++++++++++------------------
 tests/jdb.vcf               |   2 +-
 tests/tlexer.nim            |   6 +-
 tests/tvcard3.nim           |  62 +++++++-----
 vcard.nimble                |   2 +-
 6 files changed, 212 insertions(+), 196 deletions(-)

diff --git a/src/vcard/private/lexer.nim b/src/vcard/private/lexer.nim
index e95fdb4..d159b20 100644
--- a/src/vcard/private/lexer.nim
+++ b/src/vcard/private/lexer.nim
@@ -180,11 +180,6 @@ proc getColNumber*(vcl: VCardLexer, pos: int): int =
   if vcl.lineStart < pos: return pos - vcl.lineStart
   else: return (vcl.buffer.len - vcl.lineStart) + pos
 
-## Unit Tests
-## ============================================================================
-
-import std/unittest
-
 proc dumpLexerState*(l: VCardLexer): string =
   result =
     "pos        = " & $l.pos & "\p" &
@@ -195,7 +190,9 @@ proc dumpLexerState*(l: VCardLexer): string =
     "bufEnd     = " & $l.bufEnd & "\p" &
     "buffer     = " & l.buffer & "\p"
 
-suite "vcard/lexer":
+## Unit Tests
+## ============================================================================
+proc runVcardLexerPrivateTests*() =
 
   const longTestString =
     "This is my test string. There are many like it but this one is mine."
@@ -212,36 +209,34 @@ suite "vcard/lexer":
         return false
     return true
 
-  #test "fillBuffer doesn't double the buffer needlessly":
-  #  var l: VCardLexer
-
   proc readExpected(vcl: var VCardLexer, s: string): bool =
     for i in 0..<s.len:
       if vcl.read != s[i]:
         return false
     return true
 
-  test "can open and fill buffer":
+  # "can open and fill buffer":
+  block:
     var l: VCardLexer
     l.open(newStringStream("test"))
-    check:
-      l.bufferIs("test")
-      not l.isFull
-      l.readExpected("test")
+    assert l.bufferIs("test")
+    assert not l.isFull
+    assert l.readExpected("test")
 
-  test "refills buffer when emptied":
+  # "refills buffer when emptied":
+  block:
     var l: VCardLexer
     l.open(newStringStream("test"), 3)
-    check:
-      l.bufferIs("te")
-      l.isFull
-      l.read == 't'
-      l.read == 'e'
-      l.read == 's'
-      l.bufferIs("st")
-      l.read == 't'
+    assert l.bufferIs("te")
+    assert l.isFull
+    assert l.read == 't'
+    assert l.read == 'e'
+    assert l.read == 's'
+    assert l.bufferIs("st")
+    assert l.read == 't'
 
-  test "isFull correctness":
+  # "isFull correctness":
+  block:
     var l = VCardLexer(
       pos: 0,
       bookmark: -1,
@@ -251,104 +246,102 @@ suite "vcard/lexer":
 
     # s                 e
     # 0 1 2 3 4 5 6 7 8 9
-    check l.isFull
+    assert l.isFull
 
     # s p               e
     # 0 1 2 3 4 5 6 7 8 9
     discard l.read
-    check not l.isFull
+    assert not l.isFull
 
     #     e s
     # 0 1 2 3 4 5 6 7 8 9
     l.bufStart = 3
     l.pos = 3
     l.bufEnd = 2
-    check l.isFull
+    assert l.isFull
 
     #     e s p
     # 0 1 2 3 4 5 6 7 8 9
     discard l.read
-    check:
-      l.pos == 4
-      not l.isFull
+    assert l.pos == 4
+    assert not l.isFull
 
     #                 e s
     # 0 1 2 3 4 5 6 7 8 9
     l.bufStart = 9
     l.pos = 9
     l.bufEnd = 8
-    check l.isFull
+    assert l.isFull
 
     # p               e s
     # 0 1 2 3 4 5 6 7 8 9
     discard l.read
-    check:
-      l.pos == 0
-      not l.isFull
+    assert l.pos == 0
+    assert not l.isFull
 
-  test "handles wrapped lines":
+  # "handles wrapped lines":
+  block:
     var l: VCardLexer
     l.open(newStringStream("line\r\n  wrap\r\nline 2"), 3)
 
-    check l.readExpected("line wrap\r\nline 2")
+    assert l.readExpected("line wrap\r\nline 2")
 
-  test "fillBuffer correctness":
+  # "fillBuffer correctness":
+  block:
     var l: VCardLexer
     l.open(newStringStream(longTestString), 5)
-    check:
-      l.bufferIs(longTestString[0..<4])
-      l.isFull
-      l.bufStart == 0
-      l.bufEnd == 4
-      l.pos == 0
-      l.readExpected("Th")
-      not l.isFull
-      not l.atEnd
-      l.pos == 2
+    assert l.bufferIs(longTestString[0..<4])
+    assert l.isFull
+    assert l.bufStart == 0
+    assert l.bufEnd == 4
+    assert l.pos == 0
+    assert l.readExpected("Th")
+    assert not l.isFull
+    assert not l.atEnd
+    assert l.pos == 2
 
     l.fillBuffer
-    check:
-      l.isFull
-      l.bufEnd == 1
-      l.pos == 2
-      l.bufStart == 2
+    assert l.isFull
+    assert l.bufEnd == 1
+    assert l.pos == 2
+    assert l.bufStart == 2
 
-  test "bookmark preserves the buffer":
+  # "bookmark preserves the buffer":
+  block:
     var l: VCardLexer
     l.open(newStringStream(longTestString), 7)
-    check:
-      l.buffer.len == 7
-      l.bufferIs(longTestString[0..<6])
-      l.isFull
-      l.bufEnd == 6
-      l.pos == 0
-      l.bookmark == -1
-      l.readExpected(longTestString[0..<5])
-      not l.isFull
-      not l.atEnd
-      l.pos == 5
+    assert l.buffer.len == 7
+    assert l.bufferIs(longTestString[0..<6])
+    assert l.isFull
+    assert l.bufEnd == 6
+    assert l.pos == 0
+    assert l.bookmark == -1
+    assert l.readExpected(longTestString[0..<5])
+    assert not l.isFull
+    assert not l.atEnd
+    assert l.pos == 5
 
     l.setBookmark
     # read enough to require us to refill the buffer.
-    check:
-      l.bookmark == 5
-      l.readExpected(longTestString[5..<10])
-      l.pos == 3
-      newStartIdx(l) == 5
-      l.buffer.len == 7
+    assert l.bookmark == 5
+    assert l.readExpected(longTestString[5..<10])
+    assert l.pos == 3
+    assert newStartIdx(l) == 5
+    assert l.buffer.len == 7
 
     l.returnToBookmark
-    check:
-      l.bookmark == -1
-      l.pos == 5
+    assert l.bookmark == -1
+    assert l.pos == 5
 
-  test "readRune":
+  # "readRune":
+  block:
     var l: VCardLexer
     l.open(newStringStream("TEST"))
-    check:
-      l.bufferIs("TEST")
-      l.peekRune == Rune('T')
-      l.readRune == Rune('T')
-      l.readRune == Rune('E')
-      l.readRune == Rune('S')
-      l.readRune == Rune('T')
+    assert l.bufferIs("TEST")
+    assert l.peekRune == Rune('T')
+    assert l.readRune == Rune('T')
+    assert l.readRune == Rune('E')
+    assert l.readRune == Rune('S')
+    assert l.readRune == Rune('T')
+
+when isMainModule: runVcardLexerTests()
diff --git a/src/vcard/vcard3.nim b/src/vcard/vcard3.nim
index d1d48c4..dea32d1 100644
--- a/src/vcard/vcard3.nim
+++ b/src/vcard/vcard3.nim
@@ -14,7 +14,7 @@ import std/[base64, macros, options, sequtils, streams, strutils, times,
 
 import zero_functional
 
-import vcard/private/[util, lexer]
+import ./vcard/private/[util, lexer]
 
 type
   VC3_ValueTypes = enum
@@ -381,7 +381,7 @@ func newVC3_Tel*(
   telType = @[$ttVoice],
   group = none[string]()): VC3_Tel =
 
-  return VC3_Tel(name: "TEL", telType: telType, group: group)
+  return assignFields(VC3_Tel(name: "TEL"), value, telType, group)
 
 func newVC3_Email*(
   value: string,
@@ -1712,7 +1712,7 @@ proc parseContentLines(p: var VC3Parser): seq[VC3_Content] =
           lat = parseFloat(partsStr[0]),
           long = parseFloat(partsStr[1])
         ))
-      except:
+      except ValueError:
         p.error("expected two float values separated by ';' for the GEO " &
           "content type but received '" & rawValue & "'")
 
@@ -1881,144 +1881,147 @@ stateDiagram-v2
 
 ## Private Function Unit Tests
 ## ============================================================================
-
-import std/unittest
-
-suite "vcard/vcard3/private":
+proc runVcard3PrivateTests*() =
 
   proc initParser(input: string): VC3Parser =
     result = VC3Parser(filename: "private unittests")
     lexer.open(result, newStringStream(input))
 
-  test "readGroup with group":
+  # "vcard/vcard3/private"
+  block:
+
     var p = initParser("mygroup.BEGIN:VCARD")
     let g = p.readGroup
+    assert g.isSome
+    assert g.get == "mygroup"
 
-    check:
-      g.isSome
-      g.get == "mygroup"
-
-  test "readGroup without group":
+  # "readGroup without group":
+  block:
     var p = initParser("BEGIN:VCARD")
-    check p.readGroup.isNone
+    assert p.readGroup.isNone
 
-  test "expect (case-sensitive)":
+  # "expect (case-sensitive)":
+  block:
     var p = initParser("BEGIN:VCARD")
     p.expect("BEGIN", true)
 
     try:
       p.expect(":vcard", true)
-      check "" == "expect should have raised an error"
-    except: discard
+      assert "" == "expect should have raised an error"
+    except CatchableError: discard
 
-  test "expect (case-insensitive)":
+  # "expect (case-insensitive)":
+  block:
     var p = initParser("BEGIN:VCARD")
     p.expect("begin")
 
     try:
       p.expect("begin")
-      check "" == "expect should have raised an error"
-    except: discard
+      assert "" == "expect should have raised an error"
+    except CatchableError: discard
 
-  test "readName":
+  # "readName":
+  block:
     var p = initParser("TEL;tel;x-Example;x-Are1+Name")
-    check:
-      p.readName == "TEL"
-      p.read == ';'
-      p.readName == "TEL"
-      p.read == ';'
-      p.readName == "X-EXAMPLE"
-      p.read == ';'
-      p.readName == "X-ARE1"
+    assert p.readName == "TEL"
+    assert p.read == ';'
+    assert p.readName == "TEL"
+    assert p.read == ';'
+    assert p.readName == "X-EXAMPLE"
+    assert p.read == ';'
+    assert p.readName == "X-ARE1"
 
     try:
       discard p.readName
-      check "" == "readName should have raised an error"
-    except: discard
+      assert "" == "readName should have raised an error"
+    except CatchableError: discard
 
-  test "readParamValue":
+  # "readParamValue":
+  block:
     var p = initParser("TEL;TYPE=WORK;TYPE=Fun&Games%:+15551234567")
-    check:
-      p.readName == "TEL"
-      p.read == ';'
-      p.readName == "TYPE"
-      p.read == '='
-      p.readParamValue == "WORK"
-      p.read == ';'
-      p.readName == "TYPE"
-      p.read == '='
-      p.readParamValue == "Fun&Games%"
+    assert p.readName == "TEL"
+    assert p.read == ';'
+    assert p.readName == "TYPE"
+    assert p.read == '='
+    assert p.readParamValue == "WORK"
+    assert p.read == ';'
+    assert p.readName == "TYPE"
+    assert p.read == '='
+    assert p.readParamValue == "Fun&Games%"
 
-  test "readParams":
+  # "readParams":
+  block:
     var p = initParser("TEL;TYPE=WORK;TYPE=Fun&Games%,Extra:+15551234567")
-    check p.readName == "TEL"
+    assert p.readName == "TEL"
     let params = p.readParams
-    check:
-      params.len == 2
-      params[0].name == "TYPE"
-      params[0].values.len == 1
-      params[0].values[0] == "WORK"
-      params[1].name == "TYPE"
-      params[1].values.len == 2
-      params[1].values[0] == "Fun&Games%"
-      params[1].values[1] == "Extra"
+    assert params.len == 2
+    assert params[0].name == "TYPE"
+    assert params[0].values.len == 1
+    assert params[0].values[0] == "WORK"
+    assert params[1].name == "TYPE"
+    assert params[1].values.len == 2
+    assert params[1].values[0] == "Fun&Games%"
+    assert params[1].values[1] == "Extra"
 
-  test "readValue":
+  # "readValue":
+  block:
     var p = initParser("TEL;TYPE=WORK:+15551234567\r\nFN:John Smith\r\n")
-    check p.skip("TEL")
+    assert p.skip("TEL")
     discard p.readParams
-    check p.read == ':'
-    check p.readValue == "+15551234567"
+    assert p.read == ':'
+    assert p.readValue == "+15551234567"
     p.expect("\r\n")
-    check p.readName == "FN"
+    assert p.readName == "FN"
     discard p.readParams
-    check p.read == ':'
-    check p.readValue == "John Smith"
+    assert p.read == ':'
+    assert p.readValue == "John Smith"
 
-  test "readTextValueList":
+  # "readTextValueList":
+  block:
     var p = initParser("Public;John;Quincey,Adams;Rev.;Esq:limited\r\n")
-    check:
-      p.readTextValueList == @["Public"]
-      p.readTextValueList(ifPrefix = some(';')) == @["John"]
-      p.readTextValueList(ifPrefix = some(';')) == @["Quincey", "Adams"]
-      p.readTextValueList(ifPrefix = some(';')) == @["Rev."]
-      p.readTextValueList(ifPrefix = some(';')) == @["Esq:limited"]
-      p.readTextValueList(ifPrefix = some(';')) == newSeq[string]()
+    assert p.readTextValueList == @["Public"]
+    assert p.readTextValueList(ifPrefix = some(';')) == @["John"]
+    assert p.readTextValueList(ifPrefix = some(';')) == @["Quincey", "Adams"]
+    assert p.readTextValueList(ifPrefix = some(';')) == @["Rev."]
+    assert p.readTextValueList(ifPrefix = some(';')) == @["Esq:limited"]
+    assert p.readTextValueList(ifPrefix = some(';')) == newSeq[string]()
 
-  test "existsWithValue":
+  # "existsWithValue":
+  block:
     var p = initParser(";TYPE=WORK;TYPE=VOICE;TYPE=CELL")
     let params = p.readParams
-    check:
-      params.existsWithValue("TYPE", "WORK")
-      params.existsWithValue("TYPE", "CELL")
-      not params.existsWithValue("TYPE", "ISDN")
+    assert params.existsWithValue("TYPE", "WORK")
+    assert params.existsWithValue("TYPE", "CELL")
+    assert not params.existsWithValue("TYPE", "ISDN")
 
-  test "getSingleValue":
+  # "getSingleValue":
+  block:
     var p = initParser(";TYPE=WORK;TYPE=VOICE;TYPE=CELL")
     let params = p.readParams
     let val = params.getSingleValue("TYPE")
-    check:
-      val.isSome
-      val.get == "WORK"
-      params.getSingleValue("VALUE").isNone
+    assert val.isSome
+    assert val.get == "WORK"
+    assert params.getSingleValue("VALUE").isNone
 
-  test "getMultipleValues":
+  # "getMultipleValues":
+  block:
     var p = initParser(";TYPE=WORK;TYPE=VOICE;TYPE=CELL")
     let params = p.readParams
-    check:
-      params.getMultipleValues("TYPE") == @["WORK", "VOICE", "CELL"]
-      params.getMultipleValues("VALUE") == newSeq[string]()
+    assert params.getMultipleValues("TYPE") == @["WORK", "VOICE", "CELL"]
+    assert params.getMultipleValues("VALUE") == newSeq[string]()
 
-  test "validateNoParameters":
+  # "validateNoParameters":
+  block:
     var p = initParser(";TYPE=WORK;TYPE=VOICE;TYPE=CELL")
     let params = p.readParams
     p.validateNoParameters(@[], "TEST")
     try:
       p.validateNoParameters(params, "TEST")
-      check "" == "validateNoParameters should have errored"
-    except: discard
+      assert "" == "validateNoParameters should have errored"
+    except CatchableError: discard
 
-  test "validateRequredParameters":
+  # "validateRequredParameters":
+  block:
     var p = initParser(";CONTEXT=word;VALUE=uri;TYPE=CELL")
     let params = p.readParams
     p.validateRequiredParameters(params,
@@ -2026,5 +2029,7 @@ suite "vcard/vcard3/private":
 
     try:
       p.validateRequiredParameters(params, [("TYPE", "VOICE")])
-      check "" == "validateRequiredParameters should have errored"
-    except: discard
+      assert "" == "validateRequiredParameters should have errored"
+    except CatchableError: discard
+
+when isMainModule: runVcard3PrivateTests()
diff --git a/tests/jdb.vcf b/tests/jdb.vcf
index 05e2784..03f461c 100755
--- a/tests/jdb.vcf
+++ b/tests/jdb.vcf
@@ -1,6 +1,6 @@
 BEGIN:VCARD
-PRODID:-//CyrusIMAP.org//Cyrus 3.7.0-alpha0-927-gf4c98c8499-fm-202208..//EN
 VERSION:3.0
+PRODID:-//CyrusIMAP.org//Cyrus 3.7.0-alpha0-927-gf4c98c8499-fm-202208..//EN
 UID:cdaf67dc-b702-41ac-9c26-bb61df3032d2
 N:Bernard;Jonathan;;;
 FN:Jonathan Bernard
diff --git a/tests/tlexer.nim b/tests/tlexer.nim
index d7da39b..9b321e1 100644
--- a/tests/tlexer.nim
+++ b/tests/tlexer.nim
@@ -1,2 +1,6 @@
 import unittest
-import vcard/private/lexer
+import ./vcard/private/lexer
+
+suite "vcard/private/lexer":
+  test "private lexer tests":
+    runVcardLexerPrivateTests()
diff --git a/tests/tvcard3.nim b/tests/tvcard3.nim
index f7d23c3..1e1f294 100644
--- a/tests/tvcard3.nim
+++ b/tests/tvcard3.nim
@@ -1,24 +1,47 @@
-import options, unittest, vcard3, zero_functional
+import options, unittest, zero_functional
+
+import ./vcard
 
 suite "vcard/vcard3":
 
-  let testVCard =
-    "BEGIN:VCARD\r\n" &
-    "VERSION:3.0\r\n" &
-    "FN:Mr. John Q. Public\\, Esq.\r\n" &
-    "N:Public;John;Quinlan;Mr.;Esq.\r\n" &
-    "END:VCARD\r\n"
+  test "vcard3/private tests":
+    runVcard3PrivateTests()
 
-  test "minimal VCard":
-    let vc = parseVCard3(testVCard)[0]
+  let jdbVCard = readFile("tests/jdb.vcf")
+  let jdb = parseVCard3(jdbVCard)[0]
+
+  test "parseVCard3":
     check:
-      vc.n.family[0] == "Public"
-      vc.n.given[0] == "John"
-      vc.fn.value == "Mr. John Q. Public\\, Esq."
+      jdb.n.family == @["Bernard"]
+      jdb.n.given == @["Jonathan"]
+      jdb.fn.value == "Jonathan Bernard"
 
-  test "serialize minimal VCard":
-    let vc = parseVCard3(testVCard)[0]
-    check $vc == testVCard
+  test "parseVCard3File":
+    let jdb = parseVCard3File("tests/jdb.vcf")[0]
+    check:
+      jdb.email.len == 7
+      jdb.email[0].value == "jonathan@jdbernard.com"
+      jdb.email[0].emailType.contains("pref")
+      jdb.fn.value == "Jonathan Bernard"
+
+  test "email is parsed correctly":
+    check:
+      jdb.email.len == 7
+      jdb.email[0].value == "jonathan@jdbernard.com"
+      jdb.email[0].emailType.contains("pref")
+      jdb.email[0].emailType.contains("home")
+      jdb.email[1].value == "jdb@jdb-software.com"
+      jdb.email[1].emailType.contains("work")
+      jdb.email[2].group.isSome
+      jdb.email[2].group.get == "email2"
+      jdb.email[6].value == "jbernard@vectra.ai"
+      jdb.email[6].emailType.contains("work")
+
+  test "tel is parsed correctly":
+    check:
+      jdb.tel.len == 2
+      jdb.tel[0].value == "(512) 777-1602"
+      jdb.tel[0].telType.contains("CELL")
 
   test "RFC2426 Author's VCards":
     let vcardsStr =
@@ -53,12 +76,3 @@ suite "vcard/vcard3":
       vcards[0].fn.value == "Frank Dawson"
       vcards[0].email.len == 2
       (vcards[0].email --> find(it.emailType.contains("PREF"))).isSome
-
-  test "Jonathan Bernard VCard":
-    #const jdbVcard = readFile("tests/jdb.vcf")
-    let jdb = parseVCard3File("tests/jdb.vcf")[0]
-    check:
-      jdb.email.len == 7
-      jdb.email[0].value == "jonathan@jdbernard.com"
-      jdb.email[0].emailType.contains("pref")
-      jdb.fn.value == "Jonathan Bernard"
diff --git a/vcard.nimble b/vcard.nimble
index ccc997f..22d7412 100644
--- a/vcard.nimble
+++ b/vcard.nimble
@@ -1,6 +1,6 @@
 # Package
 
-version       = "0.1.2"
+version       = "0.1.3"
 author        = "Jonathan Bernard"
 description   = "Nim parser for the vCard format version 3.0 (4.0 planned)."
 license       = "MIT"