From 836afd456ff99636a7c505cdddbf254884abe870 Mon Sep 17 00:00:00 2001 From: Jacek Sieka Date: Tue, 11 Apr 2023 15:32:21 +0200 Subject: [PATCH 1/2] remove chronos dependency The `chronos` backend currently does not work as it relies on nesting `waitFor` calls, which is not supported by chronos itself - as such, this PR disables the backend in testing and prints a recommendation to users to avoid it until the problem has been resolved. faststreams used without `async` continues to work as normal, which also is the most common way to use it - the dependency on chronos in the nimble file is unattractive from a user perspective since it also leads to the the downloading of bearssl and other deps. Hopefully, by the time the chronos backend can be used, we can also specify conditional dependencies in nimble files. --- faststreams.nimble | 4 ++-- faststreams/async_backend.nim | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/faststreams.nimble b/faststreams.nimble index 7dfaa0a..4ed85d5 100644 --- a/faststreams.nimble +++ b/faststreams.nimble @@ -9,7 +9,6 @@ skipDirs = @["tests"] requires "nim >= 1.2.0", "stew", - "chronos", "unittest2" let nimc = getEnv("NIMC", "nim") # Which nim compiler to use @@ -31,7 +30,8 @@ proc run(args, path: string) = task test, "Run all tests": # TODO asyncdispatch backend is broken / untested - for backend in ["-d:asyncBackend=none", "-d:asyncBackend=chronos"]: + # TODO chronos backend uses nested waitFor which is not supported + for backend in ["-d:asyncBackend=none"]: for threads in ["--threads:off", "--threads:on"]: for mode in ["-d:debug", "-d:release", "-d:danger"]: run backend & " " & threads & " " & mode, "tests/all_tests" diff --git a/faststreams/async_backend.nim b/faststreams/async_backend.nim index ef9fe79..2086e09 100644 --- a/faststreams/async_backend.nim +++ b/faststreams/async_backend.nim @@ -20,6 +20,8 @@ const when asyncBackend == "none": discard elif asyncBackend == "chronos": + {.warning: "chronos backend uses nested calls to `waitFor` which is not supported by chronos - it is not recommended to use it until this has been resolved".} + import chronos @@ -30,6 +32,8 @@ elif asyncBackend == "chronos": await f elif asyncBackend == "asyncdispatch": + {.warning: "asyncdispatch backend currently fails tests - it may or may not work as expected".} + import std/asyncdispatch From 449e68baee3fceff94aa7ffd3ce502cbb0cff162 Mon Sep 17 00:00:00 2001 From: Jacek Sieka Date: Tue, 23 May 2023 13:30:49 +0200 Subject: [PATCH 2/2] run chronos tests in CI --- .github/workflows/ci.yml | 6 +++++- faststreams.nimble | 6 ++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 514a326..9b777e3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -156,6 +156,10 @@ jobs: nim --version nimble --version nimble install -y --depsOnly - rm -f nimble.lock + env NIMLANG=c nimble test env NIMLANG=cpp nimble test + + nimble install chronos + env NIMLANG=c nimble testChronos || true + env NIMLANG=cpp nimble testChronos || true diff --git a/faststreams.nimble b/faststreams.nimble index 4ed85d5..82c90d7 100644 --- a/faststreams.nimble +++ b/faststreams.nimble @@ -36,3 +36,9 @@ task test, "Run all tests": for mode in ["-d:debug", "-d:release", "-d:danger"]: run backend & " " & threads & " " & mode, "tests/all_tests" +task testChronos, "Run chronos tests": + # TODO chronos backend uses nested waitFor which is not supported + for backend in ["-d:asyncBackend=chronos"]: + for threads in ["--threads:off", "--threads:on"]: + for mode in ["-d:debug", "-d:release", "-d:danger"]: + run backend & " " & threads & " " & mode, "tests/all_tests"