fix #14320 (tasyncawait.nim is recently very flaky) + avoid hardcoding service ports everywhere + flakyAssert (#14327)

* hotfix #14320 tasyncawait.nim is recently very flaky
* fix #14327
* add flakyAssert

testament/lib/readme.md

@@ -0,0 +1,4 @@
+This directory contains helper files used by several tests, to avoid
+code duplication, akin to a std extension tailored for testament.
+
+Some of these could later migrate to stdlib.

testament/lib/stdtest/netutils.nim

@@ -0,0 +1,12 @@
+import std/[nativesockets, asyncdispatch, os]
+
+proc bindAvailablePort*(handle: SocketHandle, port = Port(0)): Port =
+  block:
+    var name: Sockaddr_in
+    name.sin_family = typeof(name.sin_family)(toInt(AF_INET))
+    name.sin_port = htons(uint16(port))
+    name.sin_addr.s_addr = htonl(INADDR_ANY)
+    if bindAddr(handle, cast[ptr SockAddr](addr(name)),
+                sizeof(name).Socklen) < 0'i32:
+      raiseOSError(osLastError())
+  result = getLocalAddr(handle, AF_INET)[1]

testament/lib/stdtest/testutils.nim

@@ -0,0 +1,25 @@
+import std/private/miscdollars
+
+template flakyAssert*(cond: untyped, msg = "", notifySuccess = true) =
+  ## API to deal with flaky or failing tests. This avoids disabling entire tests
+  ## altogether so that at least the parts that are working are kept being
+  ## tested. This also avoids making CI fail periodically for tests known to
+  ## be flaky. Finally, for known failures, passing `notifySuccess = true` will
+  ## log that the test succeeded, which may indicate that a bug was fixed
+  ## "by accident" and should be looked into.
+  const info = instantiationInfo(-1, true)
+  const expr = astToStr(cond)
+  if cond and not notifySuccess:
+    discard # silent success
+  else:
+    var msg2 = ""
+    toLocation(msg2, info.filename, info.line, info.column)
+    if cond:
+      # a flaky test is failing, we still report it but we don't fail CI
+      msg2.add " FLAKY_SUCCESS "
+    else:
+      # a previously failing test is now passing, a pre-existing bug might've been
+      # fixed by accidend
+      msg2.add " FLAKY_FAILURE "
+    msg2.add $expr & " " & msg
+    echo msg2

tests/arc/tasyncawait.nim

@@ -3,7 +3,8 @@ discard """
   cmd: "nim c --gc:orc $file"
 """
 
-import asyncdispatch, asyncnet, nativesockets, net, strutils, os
+import asyncdispatch, asyncnet, nativesockets, net, strutils
+from stdtest/netutils import bindAvailablePort
 
 var msgCount = 0
 
@@ -44,24 +45,16 @@ proc readMessages(client: AsyncFD) {.async.} =
       else:
         doAssert false
 
-proc createServer(port: Port) {.async.} =
+proc createServer(server: AsyncFD) {.async.} =
-  var server = createAsyncNativeSocket()
-  block:
-    var name: Sockaddr_in
-    name.sin_family = typeof(name.sin_family)(toInt(AF_INET))
-    name.sin_port = htons(uint16(port))
-    name.sin_addr.s_addr = htonl(INADDR_ANY)
-    if bindAddr(server.SocketHandle, cast[ptr SockAddr](addr(name)),
-                sizeof(name).Socklen) < 0'i32:
-      raiseOSError(osLastError())
-
   discard server.SocketHandle.listen()
   while true:
     asyncCheck readMessages(await accept(server))
 
 proc main =
-  asyncCheck createServer(Port(10335))
+  let server = createAsyncNativeSocket()
-  asyncCheck launchSwarm(Port(10335))
+  let port = bindAvailablePort(server.SocketHandle)
+  asyncCheck createServer(server)
+  asyncCheck launchSwarm(port)
   while true:
     poll()
     if clientCount == swarmSize: break

tests/async/tasyncawait.nim

@@ -1,8 +1,5 @@
-discard """
+import asyncdispatch, asyncnet, nativesockets, net, strutils
-  output: "2000"
+from stdtest/netutils import bindAvailablePort
-"""
-import asyncdispatch, asyncnet, nativesockets, net, strutils, os
-
 var msgCount = 0
 
 const
@@ -42,26 +39,18 @@ proc readMessages(client: AsyncFD) {.async.} =
       else:
         doAssert false
 
-proc createServer(port: Port) {.async.} =
+proc createServer(server: AsyncFD) {.async.} =
-  var server = createAsyncNativeSocket()
-  block:
-    var name: Sockaddr_in
-    name.sin_family = typeof(name.sin_family)(toInt(AF_INET))
-    name.sin_port = htons(uint16(port))
-    name.sin_addr.s_addr = htonl(INADDR_ANY)
-    if bindAddr(server.SocketHandle, cast[ptr SockAddr](addr(name)),
-                sizeof(name).Socklen) < 0'i32:
-      raiseOSError(osLastError())
-
   discard server.SocketHandle.listen()
   while true:
     asyncCheck readMessages(await accept(server))
 
-asyncCheck createServer(Port(10335))
+let server = createAsyncNativeSocket()
-asyncCheck launchSwarm(Port(10335))
+let port = bindAvailablePort(server.SocketHandle)
+asyncCheck createServer(server)
+asyncCheck launchSwarm(port)
 while true:
   poll()
   if clientCount == swarmSize: break
 
 assert msgCount == swarmSize * messagesToSend
-echo msgCount
+doAssert msgCount == 2000

tests/async/tasyncawait_cyclebreaker.nim

@@ -2,7 +2,8 @@ discard """
   output: "20000"
   cmd: "nim c -d:nimTypeNames -d:nimCycleBreaker $file"
 """
-import asyncdispatch, asyncnet, nativesockets, net, strutils, os
+import asyncdispatch, asyncnet, nativesockets, net, strutils
+from stdtest/netutils import bindAvailablePort
 
 var msgCount = 0
 
@@ -43,23 +44,15 @@ proc readMessages(client: AsyncFD) {.async.} =
       else:
         doAssert false
 
-proc createServer(port: Port) {.async.} =
+proc createServer(server: AsyncFD) {.async.} =
-  var server = createAsyncNativeSocket()
-  block:
-    var name: Sockaddr_in
-    name.sin_family = typeof(name.sin_family)(toInt(AF_INET))
-    name.sin_port = htons(uint16(port))
-    name.sin_addr.s_addr = htonl(INADDR_ANY)
-    if bindAddr(server.SocketHandle, cast[ptr SockAddr](addr(name)),
-                sizeof(name).Socklen) < 0'i32:
-      raiseOSError(osLastError())
-
   discard server.SocketHandle.listen()
   while true:
     asyncCheck readMessages(await accept(server))
 
-asyncCheck createServer(Port(10335))
+let server = createAsyncNativeSocket()
-asyncCheck launchSwarm(Port(10335))
+let port = bindAvailablePort(server.SocketHandle)
+asyncCheck createServer(server)
+asyncCheck launchSwarm(port)
 while true:
   poll()
   GC_collectZct()

tests/async/tasyncsend4757.nim

@@ -1,24 +1,24 @@
-discard """
-output: "Finished"
-"""
-
 import asyncdispatch, asyncnet
 
-proc createServer(port: Port) {.async.} =
+var port: Port
+proc createServer() {.async.} =
   var server = newAsyncSocket()
   server.setSockOpt(OptReuseAddr, true)
-  bindAddr(server, port)
+  bindAddr(server)
+  port = getLocalAddr(server)[1]
   server.listen()
   while true:
     let client = await server.accept()
     discard await client.recvLine()
 
-asyncCheck createServer(10335.Port)
+asyncCheck createServer()
 
+var done = false
 proc f(): Future[void] {.async.} =
-  let s = newAsyncNativeSocket()
+  let s = createAsyncNativeSocket()
-  await s.connect("localhost", 10335.Port)
+  await s.connect("localhost", port)
   await s.send("123")
-  echo "Finished"
+  done = true
 
 waitFor f()
+doAssert done

tests/async/tasyncssl.nim

@@ -1,16 +1,12 @@
 discard """
   cmd: "nim $target --hints:on --define:ssl $options $file"
-  output: "500"
-  disabled: "windows"
-  target: c
-  action: compile
 """
 
-# XXX, deactivated
+import asyncdispatch, asyncnet, net, strutils
-
+import stdtest/testutils
-import asyncdispatch, asyncnet, net, strutils, os
 
 when defined(ssl):
+  var port0: Port
   var msgCount = 0
 
   const
@@ -45,25 +41,33 @@ when defined(ssl):
         else:
           doAssert false
 
-  proc createServer(port: Port) {.async.} =
+  proc createServer() {.async.} =
     let serverContext = newContext(verifyMode = CVerifyNone,
                                    certFile = "tests/testdata/mycert.pem",
                                    keyFile = "tests/testdata/mycert.pem")
     var server = newAsyncSocket()
     serverContext.wrapSocket(server)
     server.setSockOpt(OptReuseAddr, true)
-    bindAddr(server, port)
+    bindAddr(server)
+    port0 = getLocalAddr(server)[1]
     server.listen()
     while true:
       let client = await accept(server)
       serverContext.wrapConnectedSocket(client, handshakeAsServer)
       asyncCheck readMessages(client)
 
-  asyncCheck createServer(Port(10335))
+  asyncCheck createServer()
-  asyncCheck launchSwarm(Port(10335))
+  asyncCheck launchSwarm(port0)
   while true:
     poll()
     if clientCount == swarmSize: break
 
-  assert msgCount == swarmSize * messagesToSend
+  template cond(): bool = msgCount == swarmSize * messagesToSend
-  echo msgCount
+  when defined(windows):
+    # currently: msgCount == 0
+    flakyAssert cond()
+  elif defined(linux) and int.sizeof == 8:
+    # currently:  msgCount == 10
+    flakyAssert cond()
+    assert msgCount > 0
+  else: assert cond(), $msgCount

tests/async/twinasyncrw.nim

@@ -1,9 +1,6 @@
-discard """
-  output: "5000"
-"""
 when defined(windows):
   import asyncdispatch, nativesockets, net, strutils, os, winlean
-
+  from stdtest/netutils import bindAvailablePort
   var msgCount = 0
 
   const
@@ -228,29 +225,19 @@ when defined(windows):
         else:
           doAssert false
 
-  proc createServer(port: Port) {.async.} =
+  proc createServer(server: SocketHandle) {.async.} =
-    var server = createNativeSocket()
-    setBlocking(server, false)
-    block:
-      var name = Sockaddr_in()
-      name.sin_family = toInt(Domain.AF_INET).uint16
-      name.sin_port = htons(uint16(port))
-      name.sin_addr.s_addr = htonl(INADDR_ANY)
-      if bindAddr(server, cast[ptr SockAddr](addr(name)),
-                  sizeof(name).Socklen) < 0'i32:
-        raiseOSError(osLastError())
-
     discard server.listen()
     while true:
       asyncCheck readMessages(await winAccept(AsyncFD(server)))
 
-  asyncCheck createServer(Port(10335))
+  var server = createNativeSocket()
-  asyncCheck launchSwarm(Port(10335))
+  setBlocking(server, false)
+  let port = bindAvailablePort(server)
+  asyncCheck createServer(server)
+  asyncCheck launchSwarm(port)
   while true:
     poll()
     if clientCount == swarmSize: break
 
   assert msgCount == swarmSize * messagesToSend
-  echo msgCount
+  doAssert msgCount == 5000
-else:
-  echo(5000)

tests/config.nims

@@ -1,4 +1,7 @@
-switch("path", "$nim/testament/lib") # so we can `import stdtest/foo` in this dir
+switch("path", "$lib/../testament/lib")
+  # so we can `import stdtest/foo` inside tests
+  # Using $lib/../ instead of $nim/ so you can use a different nim to run tests
+  # during local testing, eg nim --lib:lib.
 
 ## prevent common user config settings to interfere with testament expectations
 ## Indifidual tests can override this if needed to test for these options.