From 9c283687a7ad67cd6c3e331eb15de1888c7193ef Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Tue, 12 May 2020 09:49:23 -0700 Subject: [PATCH 1/9] hotfix #14320 tasyncawait.nim is recently very flaky --- tests/async/tasyncawait.nim | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/async/tasyncawait.nim b/tests/async/tasyncawait.nim index a52b0953d7265..b20485a57a5f9 100644 --- a/tests/async/tasyncawait.nim +++ b/tests/async/tasyncawait.nim @@ -57,8 +57,15 @@ proc createServer(port: Port) {.async.} = while true: asyncCheck readMessages(await accept(server)) -asyncCheck createServer(Port(10335)) -asyncCheck launchSwarm(Port(10335)) +# refs https://github.com/nim-lang/Nim/issues/14320 +# tests/arc/tasyncawait.nim uses 10335 and probably explains +# `Address already in use` errro so we use a different port. This is +# just a workaround while waiting for a cleaner fix that would wait +# (with deadline) for a port to become available. +# Note that this port is already used in other tests. +let port = 10335 + 1 +asyncCheck createServer(Port(port)) +asyncCheck launchSwarm(Port(port)) while true: poll() if clientCount == swarmSize: break From 96c5faa1240faff3010df0861cf6d4ba632680fd Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Tue, 12 May 2020 18:20:07 -0700 Subject: [PATCH 2/9] _ --- tests/async/tasyncawait.nim | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/tests/async/tasyncawait.nim b/tests/async/tasyncawait.nim index b20485a57a5f9..30d9b7f622d8d 100644 --- a/tests/async/tasyncawait.nim +++ b/tests/async/tasyncawait.nim @@ -1,6 +1,3 @@ -discard """ - output: "2000" -""" import asyncdispatch, asyncnet, nativesockets, net, strutils, os var msgCount = 0 @@ -42,7 +39,7 @@ proc readMessages(client: AsyncFD) {.async.} = else: doAssert false -proc createServer(port: Port) {.async.} = +proc bindAvailablePort(port: Port): (AsyncFD, Port) = var server = createAsyncNativeSocket() block: var name: Sockaddr_in @@ -52,23 +49,20 @@ proc createServer(port: Port) {.async.} = if bindAddr(server.SocketHandle, cast[ptr SockAddr](addr(name)), sizeof(name).Socklen) < 0'i32: raiseOSError(osLastError()) + let port = getLocalAddr(server.SocketHandle, AF_INET)[1] + result = (server, port) +proc createServer(server: AsyncFD) {.async.} = discard server.SocketHandle.listen() while true: asyncCheck readMessages(await accept(server)) -# refs https://github.com/nim-lang/Nim/issues/14320 -# tests/arc/tasyncawait.nim uses 10335 and probably explains -# `Address already in use` errro so we use a different port. This is -# just a workaround while waiting for a cleaner fix that would wait -# (with deadline) for a port to become available. -# Note that this port is already used in other tests. -let port = 10335 + 1 -asyncCheck createServer(Port(port)) -asyncCheck launchSwarm(Port(port)) +let (server, port) = bindAvailablePort(0.Port) +asyncCheck createServer(server) +asyncCheck launchSwarm(port) while true: poll() if clientCount == swarmSize: break assert msgCount == swarmSize * messagesToSend -echo msgCount +doAssert msgCount == 2000 From 107098c279b1c8778535c4b710113c3bec10d712 Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Tue, 12 May 2020 19:02:22 -0700 Subject: [PATCH 3/9] fix #14327 --- testament/lib/readme.md | 4 ++++ testament/lib/stdtest/netutils.nim | 15 +++++++++++++++ tests/arc/tasyncawait.nim | 20 ++++++-------------- tests/async/tasyncawait.nim | 19 +++---------------- tests/async/tasyncawait_cyclebreaker.nim | 20 ++++++-------------- tests/config.nims | 5 ++++- 6 files changed, 38 insertions(+), 45 deletions(-) create mode 100644 testament/lib/readme.md create mode 100644 testament/lib/stdtest/netutils.nim diff --git a/testament/lib/readme.md b/testament/lib/readme.md new file mode 100644 index 0000000000000..20e8663381968 --- /dev/null +++ b/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. diff --git a/testament/lib/stdtest/netutils.nim b/testament/lib/stdtest/netutils.nim new file mode 100644 index 0000000000000..85f2a664e16c1 --- /dev/null +++ b/testament/lib/stdtest/netutils.nim @@ -0,0 +1,15 @@ +import std/[nativesockets, asyncdispatch, os] + +proc bindAvailablePort*(port = Port(0)): (AsyncFD, Port) = + 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()) + let port = getLocalAddr(server.SocketHandle, AF_INET)[1] + result = (server, port) + diff --git a/tests/arc/tasyncawait.nim b/tests/arc/tasyncawait.nim index 7135f41736347..479757811b35c 100644 --- a/tests/arc/tasyncawait.nim +++ b/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,15 @@ proc readMessages(client: AsyncFD) {.async.} = else: doAssert false -proc createServer(port: Port) {.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()) - +proc createServer(server: AsyncFD) {.async.} = discard server.SocketHandle.listen() while true: asyncCheck readMessages(await accept(server)) proc main = - asyncCheck createServer(Port(10335)) - asyncCheck launchSwarm(Port(10335)) + let (server, port) = bindAvailablePort() + asyncCheck createServer(server) + asyncCheck launchSwarm(port) while true: poll() if clientCount == swarmSize: break diff --git a/tests/async/tasyncawait.nim b/tests/async/tasyncawait.nim index 30d9b7f622d8d..173a1ba1a814b 100644 --- a/tests/async/tasyncawait.nim +++ b/tests/async/tasyncawait.nim @@ -1,5 +1,5 @@ -import asyncdispatch, asyncnet, nativesockets, net, strutils, os - +import asyncdispatch, asyncnet, nativesockets, net, strutils +from stdtest/netutils import bindAvailablePort var msgCount = 0 const @@ -39,25 +39,12 @@ proc readMessages(client: AsyncFD) {.async.} = else: doAssert false -proc bindAvailablePort(port: Port): (AsyncFD, Port) = - 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()) - let port = getLocalAddr(server.SocketHandle, AF_INET)[1] - result = (server, port) - proc createServer(server: AsyncFD) {.async.} = discard server.SocketHandle.listen() while true: asyncCheck readMessages(await accept(server)) -let (server, port) = bindAvailablePort(0.Port) +let (server, port) = bindAvailablePort() asyncCheck createServer(server) asyncCheck launchSwarm(port) while true: diff --git a/tests/async/tasyncawait_cyclebreaker.nim b/tests/async/tasyncawait_cyclebreaker.nim index 0304d4b8213a5..2ec222a4d4afd 100644 --- a/tests/async/tasyncawait_cyclebreaker.nim +++ b/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,14 @@ proc readMessages(client: AsyncFD) {.async.} = else: doAssert false -proc createServer(port: Port) {.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()) - +proc createServer(server: AsyncFD) {.async.} = discard server.SocketHandle.listen() while true: asyncCheck readMessages(await accept(server)) -asyncCheck createServer(Port(10335)) -asyncCheck launchSwarm(Port(10335)) +let (server, port) = bindAvailablePort() +asyncCheck createServer(server) +asyncCheck launchSwarm(port) while true: poll() GC_collectZct() diff --git a/tests/config.nims b/tests/config.nims index cd4ee4b08d5ba..e91c3aa4f324e 100644 --- a/tests/config.nims +++ b/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. From af4eedb2a5a4d628d7703b2f9c977b9aa1965717 Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Tue, 12 May 2020 19:24:47 -0700 Subject: [PATCH 4/9] refactor --- testament/lib/stdtest/netutils.nim | 9 +++------ tests/arc/tasyncawait.nim | 3 ++- tests/async/tasyncawait.nim | 3 ++- tests/async/tasyncawait_cyclebreaker.nim | 3 ++- tests/async/twinasyncrw.nim | 20 ++++++-------------- 5 files changed, 15 insertions(+), 23 deletions(-) diff --git a/testament/lib/stdtest/netutils.nim b/testament/lib/stdtest/netutils.nim index 85f2a664e16c1..eb913a56a93f8 100644 --- a/testament/lib/stdtest/netutils.nim +++ b/testament/lib/stdtest/netutils.nim @@ -1,15 +1,12 @@ import std/[nativesockets, asyncdispatch, os] -proc bindAvailablePort*(port = Port(0)): (AsyncFD, Port) = - var server = createAsyncNativeSocket() +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(server.SocketHandle, cast[ptr SockAddr](addr(name)), + if bindAddr(handle, cast[ptr SockAddr](addr(name)), sizeof(name).Socklen) < 0'i32: raiseOSError(osLastError()) - let port = getLocalAddr(server.SocketHandle, AF_INET)[1] - result = (server, port) - + result = getLocalAddr(handle, AF_INET)[1] diff --git a/tests/arc/tasyncawait.nim b/tests/arc/tasyncawait.nim index 479757811b35c..f29b8d2b2db95 100644 --- a/tests/arc/tasyncawait.nim +++ b/tests/arc/tasyncawait.nim @@ -51,7 +51,8 @@ proc createServer(server: AsyncFD) {.async.} = asyncCheck readMessages(await accept(server)) proc main = - let (server, port) = bindAvailablePort() + let server = createAsyncNativeSocket() + let port = bindAvailablePort(server.SocketHandle) asyncCheck createServer(server) asyncCheck launchSwarm(port) while true: diff --git a/tests/async/tasyncawait.nim b/tests/async/tasyncawait.nim index 173a1ba1a814b..aec4ce5231c7c 100644 --- a/tests/async/tasyncawait.nim +++ b/tests/async/tasyncawait.nim @@ -44,7 +44,8 @@ proc createServer(server: AsyncFD) {.async.} = while true: asyncCheck readMessages(await accept(server)) -let (server, port) = bindAvailablePort() +let server = createAsyncNativeSocket() +let port = bindAvailablePort(server.SocketHandle) asyncCheck createServer(server) asyncCheck launchSwarm(port) while true: diff --git a/tests/async/tasyncawait_cyclebreaker.nim b/tests/async/tasyncawait_cyclebreaker.nim index 2ec222a4d4afd..2229d99c0620e 100644 --- a/tests/async/tasyncawait_cyclebreaker.nim +++ b/tests/async/tasyncawait_cyclebreaker.nim @@ -49,7 +49,8 @@ proc createServer(server: AsyncFD) {.async.} = while true: asyncCheck readMessages(await accept(server)) -let (server, port) = bindAvailablePort() +let server = createAsyncNativeSocket() +let port = bindAvailablePort(server.SocketHandle) asyncCheck createServer(server) asyncCheck launchSwarm(port) while true: diff --git a/tests/async/twinasyncrw.nim b/tests/async/twinasyncrw.nim index 352c53b4102a2..2cdc143cd1139 100644 --- a/tests/async/twinasyncrw.nim +++ b/tests/async/twinasyncrw.nim @@ -228,24 +228,16 @@ when defined(windows): else: doAssert false - proc createServer(port: Port) {.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()) - + proc createServer(server: SocketHandle) {.async.} = discard server.listen() while true: asyncCheck readMessages(await winAccept(AsyncFD(server))) - asyncCheck createServer(Port(10335)) - asyncCheck launchSwarm(Port(10335)) + var server = createNativeSocket() + setBlocking(server, false) + let port = bindAvailablePort(server) + asyncCheck createServer(server) + asyncCheck launchSwarm(port) while true: poll() if clientCount == swarmSize: break From e26cfde2ac6c50dbc5163297f8af9404f957ecda Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Tue, 12 May 2020 19:39:35 -0700 Subject: [PATCH 5/9] reactivate a deactivated test --- tests/async/tasyncsend4757.nim | 12 +++++++----- tests/async/tasyncssl.nim | 17 ++++++++--------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/tests/async/tasyncsend4757.nim b/tests/async/tasyncsend4757.nim index a87c5df9594a6..279f9daf3e9ca 100644 --- a/tests/async/tasyncsend4757.nim +++ b/tests/async/tasyncsend4757.nim @@ -4,20 +4,22 @@ 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() proc f(): Future[void] {.async.} = - let s = newAsyncNativeSocket() - await s.connect("localhost", 10335.Port) + let s = createAsyncNativeSocket() + await s.connect("localhost", port) await s.send("123") echo "Finished" diff --git a/tests/async/tasyncssl.nim b/tests/async/tasyncssl.nim index 88a5eb32ec159..6989053721ae9 100644 --- a/tests/async/tasyncssl.nim +++ b/tests/async/tasyncssl.nim @@ -2,15 +2,13 @@ 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, os +import asyncdispatch, asyncnet, net, strutils when defined(ssl): + var port0: Port + var msgCount = 0 const @@ -45,22 +43,23 @@ 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 launchSwarm(Port(10335)) + asyncCheck createServer() + asyncCheck launchSwarm(port0) while true: poll() if clientCount == swarmSize: break From fc989ccd5d3a7731065eb58da8b175c358a4ab69 Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Tue, 12 May 2020 19:59:14 -0700 Subject: [PATCH 6/9] fixup --- tests/async/tasyncsend4757.nim | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/async/tasyncsend4757.nim b/tests/async/tasyncsend4757.nim index 279f9daf3e9ca..29873a9054f46 100644 --- a/tests/async/tasyncsend4757.nim +++ b/tests/async/tasyncsend4757.nim @@ -1,7 +1,3 @@ -discard """ -output: "Finished" -""" - import asyncdispatch, asyncnet var port: Port @@ -17,10 +13,12 @@ proc createServer() {.async.} = asyncCheck createServer() +var done = false proc f(): Future[void] {.async.} = let s = createAsyncNativeSocket() await s.connect("localhost", port) await s.send("123") - echo "Finished" + done = true waitFor f() +doAssert done From 1b293ce4203e1a7e6675b1ebeeb088124328ff12 Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Tue, 12 May 2020 21:01:09 -0700 Subject: [PATCH 7/9] fix test --- tests/async/twinasyncrw.nim | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/tests/async/twinasyncrw.nim b/tests/async/twinasyncrw.nim index 2cdc143cd1139..69b092607e32d 100644 --- a/tests/async/twinasyncrw.nim +++ b/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 @@ -243,6 +240,4 @@ when defined(windows): if clientCount == swarmSize: break assert msgCount == swarmSize * messagesToSend - echo msgCount -else: - echo(5000) + doAssert msgCount == 5000 From 739fe52352d19f2ab05f2af4d527b61893ca19df Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Tue, 12 May 2020 21:08:35 -0700 Subject: [PATCH 8/9] fixup --- tests/async/tasyncssl.nim | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/async/tasyncssl.nim b/tests/async/tasyncssl.nim index 6989053721ae9..11cc5380e1f70 100644 --- a/tests/async/tasyncssl.nim +++ b/tests/async/tasyncssl.nim @@ -1,14 +1,13 @@ discard """ cmd: "nim $target --hints:on --define:ssl $options $file" - output: "500" - disabled: "windows" """ +# disabled: "windows" +# seems to fail on linux64, not linux32 import asyncdispatch, asyncnet, net, strutils when defined(ssl): var port0: Port - var msgCount = 0 const @@ -64,5 +63,4 @@ when defined(ssl): poll() if clientCount == swarmSize: break - assert msgCount == swarmSize * messagesToSend - echo msgCount + assert msgCount == swarmSize * messagesToSend, $msgCount From 3481f10936358810067b31328f0790c6fdaba5f3 Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Tue, 12 May 2020 23:41:25 -0700 Subject: [PATCH 9/9] add flakyAssert --- testament/lib/stdtest/testutils.nim | 25 +++++++++++++++++++++++++ tests/async/tasyncssl.nim | 13 ++++++++++--- 2 files changed, 35 insertions(+), 3 deletions(-) create mode 100644 testament/lib/stdtest/testutils.nim diff --git a/testament/lib/stdtest/testutils.nim b/testament/lib/stdtest/testutils.nim new file mode 100644 index 0000000000000..8083a63e8174f --- /dev/null +++ b/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 diff --git a/tests/async/tasyncssl.nim b/tests/async/tasyncssl.nim index 11cc5380e1f70..c948ee9b7b50e 100644 --- a/tests/async/tasyncssl.nim +++ b/tests/async/tasyncssl.nim @@ -1,10 +1,9 @@ discard """ cmd: "nim $target --hints:on --define:ssl $options $file" """ -# disabled: "windows" -# seems to fail on linux64, not linux32 import asyncdispatch, asyncnet, net, strutils +import stdtest/testutils when defined(ssl): var port0: Port @@ -63,4 +62,12 @@ when defined(ssl): poll() if clientCount == swarmSize: break - assert msgCount == swarmSize * messagesToSend, $msgCount + template cond(): bool = msgCount == swarmSize * messagesToSend + 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