* [Tarantool-patches] [PATCH 1/3] test: tarantool -e test add more details and avoid busy loop
2020-12-29 16:25 [Tarantool-patches] [PATCH 0/3] test: -e assert(false) test fixup Artem Starshov
@ 2020-12-29 16:25 ` Artem Starshov
2020-12-29 16:53 ` Leonid Vasiliev
2020-12-29 20:11 ` Alexander Turenko
2020-12-29 16:25 ` [Tarantool-patches] [PATCH 2/3] github-ci: add init option for containers Artem Starshov
` (3 subsequent siblings)
4 siblings, 2 replies; 13+ messages in thread
From: Artem Starshov @ 2020-12-29 16:25 UTC (permalink / raw)
To: Alexander Turenko; +Cc: tarantool-patches
From: Alexander Turenko <alexander.turenko@tarantool.org>
Background: the test fails on waiting the process termination, but
surprisingly only on GitHub hosted runners.
My guess is that the process becomes zombie, but we don't collect it.
A busy loop is not good in general. It is better to pass execution to
other fibers if there is nothing to do. This way the waiting will not
eat 100% CPU time.
Added collect more details in tarantool -e 'assert(false)' test.
---
.../gh-4983-tnt-e-assert-false-hangs.test.lua | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/test/app-tap/gh-4983-tnt-e-assert-false-hangs.test.lua b/test/app-tap/gh-4983-tnt-e-assert-false-hangs.test.lua
index 656fe212b..ff3281e0f 100755
--- a/test/app-tap/gh-4983-tnt-e-assert-false-hangs.test.lua
+++ b/test/app-tap/gh-4983-tnt-e-assert-false-hangs.test.lua
@@ -31,6 +31,7 @@ local function wait_process_completion(pid, timeout)
process_completed = true
break
end
+ fiber.sleep(0.01)
end
return process_completed
end
@@ -74,8 +75,18 @@ local res = tap.test('gh-4983-tnt-e-assert-false-hangs', function(test)
local process_completed = wait_process_completion(pid,
process_waiting_timeout)
+ local details
+ pcall(function()
+ details = {
+ cmdline = fio.open(('/proc/%d/cmdline'):format(pid),
+ {'O_RDONLY'}):read(1000000),
+ status = fio.open(('/proc/%d/status'):format(pid),
+ {'O_RDONLY'}):read(1000000),
+ }
+ end)
test:ok(process_completed,
- ('tarantool process with pid = %d completed'):format(pid))
+ ('tarantool process with pid = %d completed'):format(pid),
+ details)
-- Kill process if hangs.
if not process_completed then ffi.C.kill(pid, 9) end
--
2.28.0
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Tarantool-patches] [PATCH 1/3] test: tarantool -e test add more details and avoid busy loop
2020-12-29 16:25 ` [Tarantool-patches] [PATCH 1/3] test: tarantool -e test add more details and avoid busy loop Artem Starshov
@ 2020-12-29 16:53 ` Leonid Vasiliev
2020-12-29 20:11 ` Alexander Turenko
1 sibling, 0 replies; 13+ messages in thread
From: Leonid Vasiliev @ 2020-12-29 16:53 UTC (permalink / raw)
To: Artem Starshov, Alexander Turenko; +Cc: tarantool-patches
Hi! Thank you for the patch.
See a comment bellow:
On 29.12.2020 19:25, Artem Starshov wrote:
> From: Alexander Turenko <alexander.turenko@tarantool.org>
>
> Background: the test fails on waiting the process termination, but
> surprisingly only on GitHub hosted runners.
>
> My guess is that the process becomes zombie, but we don't collect it.
>
> A busy loop is not good in general. It is better to pass execution to
> other fibers if there is nothing to do. This way the waiting will not
> eat 100% CPU time.
>
> Added collect more details in tarantool -e 'assert(false)' test.
> ---
> .../gh-4983-tnt-e-assert-false-hangs.test.lua | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/test/app-tap/gh-4983-tnt-e-assert-false-hangs.test.lua b/test/app-tap/gh-4983-tnt-e-assert-false-hangs.test.lua
> index 656fe212b..ff3281e0f 100755
> --- a/test/app-tap/gh-4983-tnt-e-assert-false-hangs.test.lua
> +++ b/test/app-tap/gh-4983-tnt-e-assert-false-hangs.test.lua
> @@ -31,6 +31,7 @@ local function wait_process_completion(pid, timeout)
> process_completed = true
> break
> end
> + fiber.sleep(0.01)
> end
> return process_completed
> end
> @@ -74,8 +75,18 @@ local res = tap.test('gh-4983-tnt-e-assert-false-hangs', function(test)
> local process_completed = wait_process_completion(pid,
> process_waiting_timeout)
>
> + local details
It looks like the details are only interesting in case of failure. Maybe
we will collect it only in this case?
> + pcall(function()
> + details = {
> + cmdline = fio.open(('/proc/%d/cmdline'):format(pid),
> + {'O_RDONLY'}):read(1000000),
> + status = fio.open(('/proc/%d/status'):format(pid),
> + {'O_RDONLY'}):read(1000000),
> + }
> + end)
> test:ok(process_completed,
> - ('tarantool process with pid = %d completed'):format(pid))
> + ('tarantool process with pid = %d completed'):format(pid),
> + details)
>
> -- Kill process if hangs.
> if not process_completed then ffi.C.kill(pid, 9) end
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Tarantool-patches] [PATCH 1/3] test: tarantool -e test add more details and avoid busy loop
2020-12-29 16:25 ` [Tarantool-patches] [PATCH 1/3] test: tarantool -e test add more details and avoid busy loop Artem Starshov
2020-12-29 16:53 ` Leonid Vasiliev
@ 2020-12-29 20:11 ` Alexander Turenko
1 sibling, 0 replies; 13+ messages in thread
From: Alexander Turenko @ 2020-12-29 20:11 UTC (permalink / raw)
To: Artem Starshov; +Cc: tarantool-patches
On Tue, Dec 29, 2020 at 07:25:33PM +0300, Artem Starshov wrote:
> From: Alexander Turenko <alexander.turenko@tarantool.org>
I would prefer if you would send patches from youself.
>
> Background: the test fails on waiting the process termination, but
> surprisingly only on GitHub hosted runners.
>
> My guess is that the process becomes zombie, but we don't collect it.
I wrote this as brief notes for my experiments, then we observed that it
is not so. The process is not a direct child of our one.
>
> A busy loop is not good in general. It is better to pass execution to
> other fibers if there is nothing to do. This way the waiting will not
> eat 100% CPU time.
>
> Added collect more details in tarantool -e 'assert(false)' test.
> ---
> .../gh-4983-tnt-e-assert-false-hangs.test.lua | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/test/app-tap/gh-4983-tnt-e-assert-false-hangs.test.lua b/test/app-tap/gh-4983-tnt-e-assert-false-hangs.test.lua
> index 656fe212b..ff3281e0f 100755
> --- a/test/app-tap/gh-4983-tnt-e-assert-false-hangs.test.lua
> +++ b/test/app-tap/gh-4983-tnt-e-assert-false-hangs.test.lua
> @@ -31,6 +31,7 @@ local function wait_process_completion(pid, timeout)
> process_completed = true
> break
> end
> + fiber.sleep(0.01)
> end
> return process_completed
> end
> @@ -74,8 +75,18 @@ local res = tap.test('gh-4983-tnt-e-assert-false-hangs', function(test)
> local process_completed = wait_process_completion(pid,
> process_waiting_timeout)
>
> + local details
> + pcall(function()
> + details = {
> + cmdline = fio.open(('/proc/%d/cmdline'):format(pid),
> + {'O_RDONLY'}):read(1000000),
> + status = fio.open(('/proc/%d/status'):format(pid),
> + {'O_RDONLY'}):read(1000000),
> + }
> + end)
It is Linux specific, so the change is dubious.
> test:ok(process_completed,
> - ('tarantool process with pid = %d completed'):format(pid))
> + ('tarantool process with pid = %d completed'):format(pid),
> + details)
>
> -- Kill process if hangs.
> if not process_completed then ffi.C.kill(pid, 9) end
> --
> 2.28.0
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Tarantool-patches] [PATCH 2/3] github-ci: add init option for containers
2020-12-29 16:25 [Tarantool-patches] [PATCH 0/3] test: -e assert(false) test fixup Artem Starshov
2020-12-29 16:25 ` [Tarantool-patches] [PATCH 1/3] test: tarantool -e test add more details and avoid busy loop Artem Starshov
@ 2020-12-29 16:25 ` Artem Starshov
2020-12-29 16:57 ` Leonid Vasiliev
` (2 more replies)
2020-12-29 16:25 ` [Tarantool-patches] [PATCH 3/3] test: new version for -e assert(false) test Artem Starshov
` (2 subsequent siblings)
4 siblings, 3 replies; 13+ messages in thread
From: Artem Starshov @ 2020-12-29 16:25 UTC (permalink / raw)
To: Alexander Turenko; +Cc: tarantool-patches
From: Alexander Turenko <alexander.turenko@tarantool.org>
Now we have a PID 1 zombie reaping problem, when zombie
processes launched in Docker aren't collected by init,
how it would be done if we launch it on Unix.
It is fixed by adding --init option, when image launched.
Co-authored-by: Artem Starshov <artemreyt@tarantool.org>
---
.github/workflows/coverity.yml | 6 +++++-
.github/workflows/debian_11.yml | 1 +
.github/workflows/debug_coverage.yml | 6 +++++-
.github/workflows/luacheck.yml | 6 +++++-
.github/workflows/release.yml | 6 +++++-
.github/workflows/release_asan_clang11.yml | 6 +++++-
.github/workflows/release_clang.yml | 6 +++++-
.github/workflows/release_lto.yml | 6 +++++-
.github/workflows/release_lto_clang11.yml | 6 +++++-
9 files changed, 41 insertions(+), 8 deletions(-)
diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
index c8f370033..a265b3dab 100644
--- a/.github/workflows/coverity.yml
+++ b/.github/workflows/coverity.yml
@@ -22,7 +22,11 @@ jobs:
# Image built by .gitlab.mk instructions and targets from .travis.mk.
# Also additional installation of coverity tool installation check
# exists in target deps_coverity_debian at .travis.mk file.
- container: docker.io/tarantool/testing:debian-buster
+ container:
+ image: docker.io/tarantool/testing:debian-buster
+ # We lean on reaping processes by the init process (PID 1)
+ # in tests.
+ options: '--init'
steps:
- uses: actions/checkout@v1
diff --git a/.github/workflows/debian_11.yml b/.github/workflows/debian_11.yml
index 06008c231..84db2644d 100644
--- a/.github/workflows/debian_11.yml
+++ b/.github/workflows/debian_11.yml
@@ -24,6 +24,7 @@ jobs:
submodules: recursive
- name: packaging
env:
+ PACKPACK_EXTRA_DOCKER_RUN_PARAMS: '--init'
AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}
AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
AWS_S3_ENDPOINT_URL: ${{ secrets.AWS_S3_ENDPOINT_URL }}
diff --git a/.github/workflows/debug_coverage.yml b/.github/workflows/debug_coverage.yml
index e671484d7..6787a2e91 100644
--- a/.github/workflows/debug_coverage.yml
+++ b/.github/workflows/debug_coverage.yml
@@ -18,7 +18,11 @@ jobs:
fail-fast: false
# image built by .gitlab.mk instructions and targets from .travis.mk
- container: docker.io/tarantool/testing:debian-stretch
+ container:
+ image: docker.io/tarantool/testing:debian-stretch
+ # We lean on reaping processes by the init process (PID 1)
+ # in tests.
+ options: '--init'
steps:
- uses: actions/checkout@v1
diff --git a/.github/workflows/luacheck.yml b/.github/workflows/luacheck.yml
index 0a5af13b3..511265466 100644
--- a/.github/workflows/luacheck.yml
+++ b/.github/workflows/luacheck.yml
@@ -18,7 +18,11 @@ jobs:
fail-fast: false
# image built by .gitlab.mk instructions and targets from .travis.mk
- container: docker.io/tarantool/testing:debian-stretch
+ container:
+ image: docker.io/tarantool/testing:debian-stretch
+ # We lean on reaping processes by the init process (PID 1)
+ # in tests.
+ options: '--init'
steps:
- uses: actions/checkout@v1
diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
index ebd85cba3..b6fea6dbc 100644
--- a/.github/workflows/release.yml
+++ b/.github/workflows/release.yml
@@ -18,7 +18,11 @@ jobs:
fail-fast: false
# image built by .gitlab.mk instructions and targets from .travis.mk
- container: docker.io/tarantool/testing:debian-stretch
+ container:
+ image: docker.io/tarantool/testing:debian-stretch
+ # We lean on reaping processes by the init process (PID 1)
+ # in tests.
+ options: '--init'
steps:
- uses: actions/checkout@v1
diff --git a/.github/workflows/release_asan_clang11.yml b/.github/workflows/release_asan_clang11.yml
index a3885bc27..346f9f0ee 100644
--- a/.github/workflows/release_asan_clang11.yml
+++ b/.github/workflows/release_asan_clang11.yml
@@ -18,7 +18,11 @@ jobs:
fail-fast: false
# image built by .gitlab.mk instructions and targets from .travis.mk
- container: docker.io/tarantool/testing:debian-buster
+ container:
+ image: docker.io/tarantool/testing:debian-buster
+ # We lean on reaping processes by the init process (PID 1)
+ # in tests.
+ options: '--init'
steps:
- uses: actions/checkout@v2.3.4
diff --git a/.github/workflows/release_clang.yml b/.github/workflows/release_clang.yml
index a94c8840a..42decd5da 100644
--- a/.github/workflows/release_clang.yml
+++ b/.github/workflows/release_clang.yml
@@ -18,7 +18,11 @@ jobs:
fail-fast: false
# image built by .gitlab.mk instructions and targets from .travis.mk
- container: docker.io/tarantool/testing:debian-stretch
+ container:
+ image: docker.io/tarantool/testing:debian-stretch
+ # We lean on reaping processes by the init process (PID 1)
+ # in tests.
+ options: '--init'
steps:
- uses: actions/checkout@v1
diff --git a/.github/workflows/release_lto.yml b/.github/workflows/release_lto.yml
index c9905f226..526f19974 100644
--- a/.github/workflows/release_lto.yml
+++ b/.github/workflows/release_lto.yml
@@ -18,7 +18,11 @@ jobs:
fail-fast: false
# image built by .gitlab.mk instructions and targets from .travis.mk
- container: docker.io/tarantool/testing:debian-buster
+ container:
+ image: docker.io/tarantool/testing:debian-buster
+ # We lean on reaping processes by the init process (PID 1)
+ # in tests.
+ options: '--init'
steps:
- uses: actions/checkout@v2.3.4
diff --git a/.github/workflows/release_lto_clang11.yml b/.github/workflows/release_lto_clang11.yml
index d57b15f3a..4a10712de 100644
--- a/.github/workflows/release_lto_clang11.yml
+++ b/.github/workflows/release_lto_clang11.yml
@@ -18,7 +18,11 @@ jobs:
fail-fast: false
# image built by .gitlab.mk instructions and targets from .travis.mk
- container: docker.io/tarantool/testing:debian-buster
+ container:
+ image: docker.io/tarantool/testing:debian-buster
+ # We lean on reaping processes by the init process (PID 1)
+ # in tests.
+ options: '--init'
steps:
- uses: actions/checkout@v2.3.4
--
2.28.0
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Tarantool-patches] [PATCH 2/3] github-ci: add init option for containers
2020-12-29 16:25 ` [Tarantool-patches] [PATCH 2/3] github-ci: add init option for containers Artem Starshov
@ 2020-12-29 16:57 ` Leonid Vasiliev
2020-12-29 18:32 ` Alexander V. Tikhonov
2020-12-29 19:25 ` Alexander Turenko
2 siblings, 0 replies; 13+ messages in thread
From: Leonid Vasiliev @ 2020-12-29 16:57 UTC (permalink / raw)
To: Artem Starshov, Alexander Turenko; +Cc: tarantool-patches
Hi! Thank you for the patch.
I think this changes are well verifiied by tests. So if the tests are
green - LGTM.
On 29.12.2020 19:25, Artem Starshov wrote:
> From: Alexander Turenko <alexander.turenko@tarantool.org>
>
> Now we have a PID 1 zombie reaping problem, when zombie
> processes launched in Docker aren't collected by init,
> how it would be done if we launch it on Unix.
>
> It is fixed by adding --init option, when image launched.
>
> Co-authored-by: Artem Starshov <artemreyt@tarantool.org>
> ---
> .github/workflows/coverity.yml | 6 +++++-
> .github/workflows/debian_11.yml | 1 +
> .github/workflows/debug_coverage.yml | 6 +++++-
> .github/workflows/luacheck.yml | 6 +++++-
> .github/workflows/release.yml | 6 +++++-
> .github/workflows/release_asan_clang11.yml | 6 +++++-
> .github/workflows/release_clang.yml | 6 +++++-
> .github/workflows/release_lto.yml | 6 +++++-
> .github/workflows/release_lto_clang11.yml | 6 +++++-
> 9 files changed, 41 insertions(+), 8 deletions(-)
>
> diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
> index c8f370033..a265b3dab 100644
> --- a/.github/workflows/coverity.yml
> +++ b/.github/workflows/coverity.yml
> @@ -22,7 +22,11 @@ jobs:
> # Image built by .gitlab.mk instructions and targets from .travis.mk.
> # Also additional installation of coverity tool installation check
> # exists in target deps_coverity_debian at .travis.mk file.
> - container: docker.io/tarantool/testing:debian-buster
> + container:
> + image: docker.io/tarantool/testing:debian-buster
> + # We lean on reaping processes by the init process (PID 1)
> + # in tests.
> + options: '--init'
>
> steps:
> - uses: actions/checkout@v1
> diff --git a/.github/workflows/debian_11.yml b/.github/workflows/debian_11.yml
> index 06008c231..84db2644d 100644
> --- a/.github/workflows/debian_11.yml
> +++ b/.github/workflows/debian_11.yml
> @@ -24,6 +24,7 @@ jobs:
> submodules: recursive
> - name: packaging
> env:
> + PACKPACK_EXTRA_DOCKER_RUN_PARAMS: '--init'
> AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}
> AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
> AWS_S3_ENDPOINT_URL: ${{ secrets.AWS_S3_ENDPOINT_URL }}
> diff --git a/.github/workflows/debug_coverage.yml b/.github/workflows/debug_coverage.yml
> index e671484d7..6787a2e91 100644
> --- a/.github/workflows/debug_coverage.yml
> +++ b/.github/workflows/debug_coverage.yml
> @@ -18,7 +18,11 @@ jobs:
> fail-fast: false
>
> # image built by .gitlab.mk instructions and targets from .travis.mk
> - container: docker.io/tarantool/testing:debian-stretch
> + container:
> + image: docker.io/tarantool/testing:debian-stretch
> + # We lean on reaping processes by the init process (PID 1)
> + # in tests.
> + options: '--init'
>
> steps:
> - uses: actions/checkout@v1
> diff --git a/.github/workflows/luacheck.yml b/.github/workflows/luacheck.yml
> index 0a5af13b3..511265466 100644
> --- a/.github/workflows/luacheck.yml
> +++ b/.github/workflows/luacheck.yml
> @@ -18,7 +18,11 @@ jobs:
> fail-fast: false
>
> # image built by .gitlab.mk instructions and targets from .travis.mk
> - container: docker.io/tarantool/testing:debian-stretch
> + container:
> + image: docker.io/tarantool/testing:debian-stretch
> + # We lean on reaping processes by the init process (PID 1)
> + # in tests.
> + options: '--init'
>
> steps:
> - uses: actions/checkout@v1
> diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
> index ebd85cba3..b6fea6dbc 100644
> --- a/.github/workflows/release.yml
> +++ b/.github/workflows/release.yml
> @@ -18,7 +18,11 @@ jobs:
> fail-fast: false
>
> # image built by .gitlab.mk instructions and targets from .travis.mk
> - container: docker.io/tarantool/testing:debian-stretch
> + container:
> + image: docker.io/tarantool/testing:debian-stretch
> + # We lean on reaping processes by the init process (PID 1)
> + # in tests.
> + options: '--init'
>
> steps:
> - uses: actions/checkout@v1
> diff --git a/.github/workflows/release_asan_clang11.yml b/.github/workflows/release_asan_clang11.yml
> index a3885bc27..346f9f0ee 100644
> --- a/.github/workflows/release_asan_clang11.yml
> +++ b/.github/workflows/release_asan_clang11.yml
> @@ -18,7 +18,11 @@ jobs:
> fail-fast: false
>
> # image built by .gitlab.mk instructions and targets from .travis.mk
> - container: docker.io/tarantool/testing:debian-buster
> + container:
> + image: docker.io/tarantool/testing:debian-buster
> + # We lean on reaping processes by the init process (PID 1)
> + # in tests.
> + options: '--init'
>
> steps:
> - uses: actions/checkout@v2.3.4
> diff --git a/.github/workflows/release_clang.yml b/.github/workflows/release_clang.yml
> index a94c8840a..42decd5da 100644
> --- a/.github/workflows/release_clang.yml
> +++ b/.github/workflows/release_clang.yml
> @@ -18,7 +18,11 @@ jobs:
> fail-fast: false
>
> # image built by .gitlab.mk instructions and targets from .travis.mk
> - container: docker.io/tarantool/testing:debian-stretch
> + container:
> + image: docker.io/tarantool/testing:debian-stretch
> + # We lean on reaping processes by the init process (PID 1)
> + # in tests.
> + options: '--init'
>
> steps:
> - uses: actions/checkout@v1
> diff --git a/.github/workflows/release_lto.yml b/.github/workflows/release_lto.yml
> index c9905f226..526f19974 100644
> --- a/.github/workflows/release_lto.yml
> +++ b/.github/workflows/release_lto.yml
> @@ -18,7 +18,11 @@ jobs:
> fail-fast: false
>
> # image built by .gitlab.mk instructions and targets from .travis.mk
> - container: docker.io/tarantool/testing:debian-buster
> + container:
> + image: docker.io/tarantool/testing:debian-buster
> + # We lean on reaping processes by the init process (PID 1)
> + # in tests.
> + options: '--init'
>
> steps:
> - uses: actions/checkout@v2.3.4
> diff --git a/.github/workflows/release_lto_clang11.yml b/.github/workflows/release_lto_clang11.yml
> index d57b15f3a..4a10712de 100644
> --- a/.github/workflows/release_lto_clang11.yml
> +++ b/.github/workflows/release_lto_clang11.yml
> @@ -18,7 +18,11 @@ jobs:
> fail-fast: false
>
> # image built by .gitlab.mk instructions and targets from .travis.mk
> - container: docker.io/tarantool/testing:debian-buster
> + container:
> + image: docker.io/tarantool/testing:debian-buster
> + # We lean on reaping processes by the init process (PID 1)
> + # in tests.
> + options: '--init'
>
> steps:
> - uses: actions/checkout@v2.3.4
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Tarantool-patches] [PATCH 2/3] github-ci: add init option for containers
2020-12-29 16:25 ` [Tarantool-patches] [PATCH 2/3] github-ci: add init option for containers Artem Starshov
2020-12-29 16:57 ` Leonid Vasiliev
@ 2020-12-29 18:32 ` Alexander V. Tikhonov
2020-12-29 19:25 ` Alexander Turenko
2 siblings, 0 replies; 13+ messages in thread
From: Alexander V. Tikhonov @ 2020-12-29 18:32 UTC (permalink / raw)
To: Artem Starshov; +Cc: tarantool-patches
Hi Artem, thanks for the patch, as I see no new degradation found in
gitlab-ci testing commit criteria pipeline [1], also it helped to fix
issues in github-ci testing, patch LGTM.
[1] - https://gitlab.com/tarantool/tarantool/-/pipelines/235528929
On Tue, Dec 29, 2020 at 07:25:34PM +0300, Artem Starshov via Tarantool-patches wrote:
> From: Alexander Turenko <alexander.turenko@tarantool.org>
>
> Now we have a PID 1 zombie reaping problem, when zombie
> processes launched in Docker aren't collected by init,
> how it would be done if we launch it on Unix.
>
> It is fixed by adding --init option, when image launched.
>
> Co-authored-by: Artem Starshov <artemreyt@tarantool.org>
> ---
> .github/workflows/coverity.yml | 6 +++++-
> .github/workflows/debian_11.yml | 1 +
> .github/workflows/debug_coverage.yml | 6 +++++-
> .github/workflows/luacheck.yml | 6 +++++-
> .github/workflows/release.yml | 6 +++++-
> .github/workflows/release_asan_clang11.yml | 6 +++++-
> .github/workflows/release_clang.yml | 6 +++++-
> .github/workflows/release_lto.yml | 6 +++++-
> .github/workflows/release_lto_clang11.yml | 6 +++++-
> 9 files changed, 41 insertions(+), 8 deletions(-)
>
> diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
> index c8f370033..a265b3dab 100644
> --- a/.github/workflows/coverity.yml
> +++ b/.github/workflows/coverity.yml
> @@ -22,7 +22,11 @@ jobs:
> # Image built by .gitlab.mk instructions and targets from .travis.mk.
> # Also additional installation of coverity tool installation check
> # exists in target deps_coverity_debian at .travis.mk file.
> - container: docker.io/tarantool/testing:debian-buster
> + container:
> + image: docker.io/tarantool/testing:debian-buster
> + # We lean on reaping processes by the init process (PID 1)
> + # in tests.
> + options: '--init'
>
> steps:
> - uses: actions/checkout@v1
> diff --git a/.github/workflows/debian_11.yml b/.github/workflows/debian_11.yml
> index 06008c231..84db2644d 100644
> --- a/.github/workflows/debian_11.yml
> +++ b/.github/workflows/debian_11.yml
> @@ -24,6 +24,7 @@ jobs:
> submodules: recursive
> - name: packaging
> env:
> + PACKPACK_EXTRA_DOCKER_RUN_PARAMS: '--init'
> AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}
> AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
> AWS_S3_ENDPOINT_URL: ${{ secrets.AWS_S3_ENDPOINT_URL }}
> diff --git a/.github/workflows/debug_coverage.yml b/.github/workflows/debug_coverage.yml
> index e671484d7..6787a2e91 100644
> --- a/.github/workflows/debug_coverage.yml
> +++ b/.github/workflows/debug_coverage.yml
> @@ -18,7 +18,11 @@ jobs:
> fail-fast: false
>
> # image built by .gitlab.mk instructions and targets from .travis.mk
> - container: docker.io/tarantool/testing:debian-stretch
> + container:
> + image: docker.io/tarantool/testing:debian-stretch
> + # We lean on reaping processes by the init process (PID 1)
> + # in tests.
> + options: '--init'
>
> steps:
> - uses: actions/checkout@v1
> diff --git a/.github/workflows/luacheck.yml b/.github/workflows/luacheck.yml
> index 0a5af13b3..511265466 100644
> --- a/.github/workflows/luacheck.yml
> +++ b/.github/workflows/luacheck.yml
> @@ -18,7 +18,11 @@ jobs:
> fail-fast: false
>
> # image built by .gitlab.mk instructions and targets from .travis.mk
> - container: docker.io/tarantool/testing:debian-stretch
> + container:
> + image: docker.io/tarantool/testing:debian-stretch
> + # We lean on reaping processes by the init process (PID 1)
> + # in tests.
> + options: '--init'
>
> steps:
> - uses: actions/checkout@v1
> diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
> index ebd85cba3..b6fea6dbc 100644
> --- a/.github/workflows/release.yml
> +++ b/.github/workflows/release.yml
> @@ -18,7 +18,11 @@ jobs:
> fail-fast: false
>
> # image built by .gitlab.mk instructions and targets from .travis.mk
> - container: docker.io/tarantool/testing:debian-stretch
> + container:
> + image: docker.io/tarantool/testing:debian-stretch
> + # We lean on reaping processes by the init process (PID 1)
> + # in tests.
> + options: '--init'
>
> steps:
> - uses: actions/checkout@v1
> diff --git a/.github/workflows/release_asan_clang11.yml b/.github/workflows/release_asan_clang11.yml
> index a3885bc27..346f9f0ee 100644
> --- a/.github/workflows/release_asan_clang11.yml
> +++ b/.github/workflows/release_asan_clang11.yml
> @@ -18,7 +18,11 @@ jobs:
> fail-fast: false
>
> # image built by .gitlab.mk instructions and targets from .travis.mk
> - container: docker.io/tarantool/testing:debian-buster
> + container:
> + image: docker.io/tarantool/testing:debian-buster
> + # We lean on reaping processes by the init process (PID 1)
> + # in tests.
> + options: '--init'
>
> steps:
> - uses: actions/checkout@v2.3.4
> diff --git a/.github/workflows/release_clang.yml b/.github/workflows/release_clang.yml
> index a94c8840a..42decd5da 100644
> --- a/.github/workflows/release_clang.yml
> +++ b/.github/workflows/release_clang.yml
> @@ -18,7 +18,11 @@ jobs:
> fail-fast: false
>
> # image built by .gitlab.mk instructions and targets from .travis.mk
> - container: docker.io/tarantool/testing:debian-stretch
> + container:
> + image: docker.io/tarantool/testing:debian-stretch
> + # We lean on reaping processes by the init process (PID 1)
> + # in tests.
> + options: '--init'
>
> steps:
> - uses: actions/checkout@v1
> diff --git a/.github/workflows/release_lto.yml b/.github/workflows/release_lto.yml
> index c9905f226..526f19974 100644
> --- a/.github/workflows/release_lto.yml
> +++ b/.github/workflows/release_lto.yml
> @@ -18,7 +18,11 @@ jobs:
> fail-fast: false
>
> # image built by .gitlab.mk instructions and targets from .travis.mk
> - container: docker.io/tarantool/testing:debian-buster
> + container:
> + image: docker.io/tarantool/testing:debian-buster
> + # We lean on reaping processes by the init process (PID 1)
> + # in tests.
> + options: '--init'
>
> steps:
> - uses: actions/checkout@v2.3.4
> diff --git a/.github/workflows/release_lto_clang11.yml b/.github/workflows/release_lto_clang11.yml
> index d57b15f3a..4a10712de 100644
> --- a/.github/workflows/release_lto_clang11.yml
> +++ b/.github/workflows/release_lto_clang11.yml
> @@ -18,7 +18,11 @@ jobs:
> fail-fast: false
>
> # image built by .gitlab.mk instructions and targets from .travis.mk
> - container: docker.io/tarantool/testing:debian-buster
> + container:
> + image: docker.io/tarantool/testing:debian-buster
> + # We lean on reaping processes by the init process (PID 1)
> + # in tests.
> + options: '--init'
>
> steps:
> - uses: actions/checkout@v2.3.4
> --
> 2.28.0
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Tarantool-patches] [PATCH 2/3] github-ci: add init option for containers
2020-12-29 16:25 ` [Tarantool-patches] [PATCH 2/3] github-ci: add init option for containers Artem Starshov
2020-12-29 16:57 ` Leonid Vasiliev
2020-12-29 18:32 ` Alexander V. Tikhonov
@ 2020-12-29 19:25 ` Alexander Turenko
2 siblings, 0 replies; 13+ messages in thread
From: Alexander Turenko @ 2020-12-29 19:25 UTC (permalink / raw)
To: Artem Starshov; +Cc: tarantool-patches
Pushed out of order to fix our CI to master, 2.6, 2.5 and 1.10.
Made several changes as listed below.
WBR, Alexander Turenko.
On Tue, Dec 29, 2020 at 07:25:34PM +0300, Artem Starshov wrote:
> From: Alexander Turenko <alexander.turenko@tarantool.org>
>
> Now we have a PID 1 zombie reaping problem, when zombie
> processes launched in Docker aren't collected by init,
> how it would be done if we launch it on Unix.
It is a bit imprecise: 'Docker' and 'Unix' are not opposites. Changed to
'launch it on a real host'.
>
> It is fixed by adding --init option, when image launched.
This is a bit imprecise too: a container is created/started, not an image.
Changed to 'when a container is created or started'. I replaced the term
'launched', because actual commands are 'docker create' or 'docker
start' (you can see it in CI logs).
>
> Co-authored-by: Artem Starshov <artemreyt@tarantool.org>
Added 'Follows up #4983'.
> ---
> .github/workflows/coverity.yml | 6 +++++-
> .github/workflows/debian_11.yml | 1 +
> .github/workflows/debug_coverage.yml | 6 +++++-
> .github/workflows/luacheck.yml | 6 +++++-
> .github/workflows/release.yml | 6 +++++-
> .github/workflows/release_asan_clang11.yml | 6 +++++-
> .github/workflows/release_clang.yml | 6 +++++-
> .github/workflows/release_lto.yml | 6 +++++-
> .github/workflows/release_lto_clang11.yml | 6 +++++-
> 9 files changed, 41 insertions(+), 8 deletions(-)
>
> diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
> index c8f370033..a265b3dab 100644
> --- a/.github/workflows/coverity.yml
> +++ b/.github/workflows/coverity.yml
> @@ -22,7 +22,11 @@ jobs:
> # Image built by .gitlab.mk instructions and targets from .travis.mk.
> # Also additional installation of coverity tool installation check
> # exists in target deps_coverity_debian at .travis.mk file.
> - container: docker.io/tarantool/testing:debian-buster
> + container:
> + image: docker.io/tarantool/testing:debian-buster
> + # We lean on reaping processes by the init process (PID 1)
> + # in tests.
> + options: '--init'
That wording was in my fast-try-do-not-merge commit.
Reworded to make it sound a bit more elegant for me (and added the test
name):
| # Our testing expects that the init process (PID 1) will
| # reap orphan processes. At least the following test leans
| # on it: app-tap/gh-4983-tnt-e-assert-false-hangs.test.lua.
>
> steps:
> - uses: actions/checkout@v1
> diff --git a/.github/workflows/debian_11.yml b/.github/workflows/debian_11.yml
> index 06008c231..84db2644d 100644
> --- a/.github/workflows/debian_11.yml
> +++ b/.github/workflows/debian_11.yml
> @@ -24,6 +24,7 @@ jobs:
> submodules: recursive
> - name: packaging
> env:
> + PACKPACK_EXTRA_DOCKER_RUN_PARAMS: '--init'
> AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}
> AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
> AWS_S3_ENDPOINT_URL: ${{ secrets.AWS_S3_ENDPOINT_URL }}
Added the comment here too.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Tarantool-patches] [PATCH 3/3] test: new version for -e assert(false) test
2020-12-29 16:25 [Tarantool-patches] [PATCH 0/3] test: -e assert(false) test fixup Artem Starshov
2020-12-29 16:25 ` [Tarantool-patches] [PATCH 1/3] test: tarantool -e test add more details and avoid busy loop Artem Starshov
2020-12-29 16:25 ` [Tarantool-patches] [PATCH 2/3] github-ci: add init option for containers Artem Starshov
@ 2020-12-29 16:25 ` Artem Starshov
2020-12-29 17:07 ` Leonid Vasiliev
2020-12-29 20:16 ` Alexander Turenko
2020-12-29 16:45 ` [Tarantool-patches] [PATCH 0/3] test: -e assert(false) test fixup Artem
2020-12-29 17:11 ` Leonid Vasiliev
4 siblings, 2 replies; 13+ messages in thread
From: Artem Starshov @ 2020-12-29 16:25 UTC (permalink / raw)
To: Alexander Turenko; +Cc: tarantool-patches
Functions with timeout transferred to module
test/app-tap/lua/process_timeout.lua for futher
using in other tests.
Before this version of test was already done,
but by mistake older version was rebased on
master.
---
.../gh-4983-tnt-e-assert-false-hangs.test.lua | 70 ++++---------------
test/app-tap/lua/process_timeout.lua | 57 +++++++++++++++
test/app-tap/suite.ini | 2 +-
3 files changed, 70 insertions(+), 59 deletions(-)
create mode 100644 test/app-tap/lua/process_timeout.lua
diff --git a/test/app-tap/gh-4983-tnt-e-assert-false-hangs.test.lua b/test/app-tap/gh-4983-tnt-e-assert-false-hangs.test.lua
index ff3281e0f..9d0f0e281 100755
--- a/test/app-tap/gh-4983-tnt-e-assert-false-hangs.test.lua
+++ b/test/app-tap/gh-4983-tnt-e-assert-false-hangs.test.lua
@@ -1,70 +1,24 @@
#!/usr/bin/env tarantool
-local fiber = require('fiber')
-local clock = require('clock')
-local ffi = require('ffi')
-local fio = require('fio')
-local errno = require('errno')
+local common = require('process_timeout')
local tap = require('tap')
+local fio = require('fio')
+local ffi = require('ffi')
--
-- gh-4983: tarantool -e 'assert(false)' hangs
--
--- For process_is_alive.
-ffi.cdef([[
- int kill(pid_t pid, int sig);
-]])
-
---- Verify whether a process is alive.
-local function process_is_alive(pid)
- local rc = ffi.C.kill(pid, 0)
- return rc == 0 or errno() ~= errno.ESRCH
-end
-
---- Returns true if process completed before timeout, false otherwise.
-local function wait_process_completion(pid, timeout)
- local start_time = clock.monotonic()
- local process_completed = false
- while clock.monotonic() - start_time < timeout do
- if not process_is_alive(pid) then
- process_completed = true
- break
- end
- fiber.sleep(0.01)
- end
- return process_completed
-end
-
---- Open file on reading with timeout.
-local function open_with_timeout(filename, timeout)
- local fh
- local start_time = clock.monotonic()
- while not fh and clock.monotonic() - start_time < timeout do
- fh = fio.open(filename, {'O_RDONLY'})
- end
- return fh
-end
-
---- Try to read from file with timeout with interval.
-local function read_with_timeout(fh, timeout, interval)
- local data = ''
- local start_time = clock.monotonic()
- while #data == 0 and clock.monotonic() - start_time < timeout do
- data = fh:read()
- if #data == 0 then fiber.sleep(interval) end
- end
- return data
-end
-
local TARANTOOL_PATH = arg[-1]
local output_file = fio.abspath('out.txt')
local line = ('%s -e "assert(false)" > %s 2>&1 & echo $!'):
format(TARANTOOL_PATH, output_file)
-local process_waiting_timeout = 30.0
-local file_read_timeout = 30.0
-local file_read_interval = 0.2
-local file_open_timeout = 30.0
+
+-- Like a default timeout for `cond_wait` in test-run
+local process_waiting_timeout = 60.0
+local file_read_timeout = 60.0
+local file_open_timeout = 60.0
+local file_read_interval = 0.01
local res = tap.test('gh-4983-tnt-e-assert-false-hangs', function(test)
test:plan(2)
@@ -72,7 +26,7 @@ local res = tap.test('gh-4983-tnt-e-assert-false-hangs', function(test)
local pid = tonumber(io.popen(line):read('*line'))
assert(pid, 'pid of proccess can\'t be recieved')
- local process_completed = wait_process_completion(pid,
+ local process_completed = common.wait_process_completion(pid,
process_waiting_timeout)
local details
@@ -91,10 +45,10 @@ local res = tap.test('gh-4983-tnt-e-assert-false-hangs', function(test)
-- Kill process if hangs.
if not process_completed then ffi.C.kill(pid, 9) end
- local fh = open_with_timeout(output_file, file_open_timeout)
+ local fh = common.open_with_timeout(output_file, file_open_timeout)
assert(fh, 'error while opening ' .. output_file)
- local data = read_with_timeout(fh, file_read_timeout, file_read_interval)
+ local data = common.read_with_timeout(fh, file_read_timeout, file_read_interval)
test:like(data, 'assertion failed', 'assertion failure is displayed')
fh:close()
diff --git a/test/app-tap/lua/process_timeout.lua b/test/app-tap/lua/process_timeout.lua
new file mode 100644
index 000000000..141fb0e43
--- /dev/null
+++ b/test/app-tap/lua/process_timeout.lua
@@ -0,0 +1,57 @@
+local fiber = require('fiber')
+local clock = require('clock')
+local ffi = require('ffi')
+local fio = require('fio')
+local errno = require('errno')
+
+-- For process_is_alive.
+ffi.cdef([[
+ int kill(pid_t pid, int sig);
+]])
+
+--- Verify whether a process is alive.
+local function process_is_alive(pid)
+ local rc = ffi.C.kill(pid, 0)
+ return rc == 0 or errno() ~= errno.ESRCH
+end
+
+--- Returns true if process completed before timeout, false otherwise.
+local function wait_process_completion(pid, timeout)
+ local start_time = clock.monotonic()
+ local process_completed = false
+ while clock.monotonic() - start_time < timeout do
+ if not process_is_alive(pid) then
+ process_completed = true
+ break
+ end
+ end
+ return process_completed
+end
+
+--- Open file on reading with timeout.
+local function open_with_timeout(filename, timeout)
+ local fh
+ local start_time = clock.monotonic()
+ while not fh and clock.monotonic() - start_time < timeout do
+ fh = fio.open(filename, {'O_RDONLY'})
+ end
+ return fh
+end
+
+--- Try to read from file with timeout with interval.
+local function read_with_timeout(fh, timeout, interval)
+ local data = ''
+ local start_time = clock.monotonic()
+ while #data == 0 and clock.monotonic() - start_time < timeout do
+ data = fh:read()
+ if #data == 0 then fiber.sleep(interval) end
+ end
+ return data
+end
+
+return {
+ process_is_alive = process_is_alive,
+ wait_process_completion = wait_process_completion,
+ open_with_timeout = open_with_timeout,
+ read_with_timeout = read_with_timeout
+}
diff --git a/test/app-tap/suite.ini b/test/app-tap/suite.ini
index 2f16128f9..6e27f88d6 100644
--- a/test/app-tap/suite.ini
+++ b/test/app-tap/suite.ini
@@ -1,7 +1,7 @@
[default]
core = app
description = application server tests (TAP)
-lua_libs = lua/require_mod.lua lua/serializer_test.lua
+lua_libs = lua/require_mod.lua lua/serializer_test.lua lua/process_timeout.lua
is_parallel = True
pretest_clean = True
use_unix_sockets_iproto = True
--
2.28.0
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Tarantool-patches] [PATCH 3/3] test: new version for -e assert(false) test
2020-12-29 16:25 ` [Tarantool-patches] [PATCH 3/3] test: new version for -e assert(false) test Artem Starshov
@ 2020-12-29 17:07 ` Leonid Vasiliev
2020-12-29 20:16 ` Alexander Turenko
1 sibling, 0 replies; 13+ messages in thread
From: Leonid Vasiliev @ 2020-12-29 17:07 UTC (permalink / raw)
To: Artem Starshov, Alexander Turenko; +Cc: tarantool-patches
Hi! Thank you for the patch.
See some comments below:
- I think you don't need to introduce a "new version". I propose to
split this patch into 2 separate patches. First - increase the timeout,
second - move common code.
On 29.12.2020 19:25, Artem Starshov wrote:
> Functions with timeout transferred to module
> test/app-tap/lua/process_timeout.lua for futher
> using in other tests.
>
> Before this version of test was already done,
> but by mistake older version was rebased on
> master.
> ---
> .../gh-4983-tnt-e-assert-false-hangs.test.lua | 70 ++++---------------
> test/app-tap/lua/process_timeout.lua | 57 +++++++++++++++
> test/app-tap/suite.ini | 2 +-
> 3 files changed, 70 insertions(+), 59 deletions(-)
> create mode 100644 test/app-tap/lua/process_timeout.lua
>
> diff --git a/test/app-tap/gh-4983-tnt-e-assert-false-hangs.test.lua b/test/app-tap/gh-4983-tnt-e-assert-false-hangs.test.lua
> index ff3281e0f..9d0f0e281 100755
> --- a/test/app-tap/gh-4983-tnt-e-assert-false-hangs.test.lua
> +++ b/test/app-tap/gh-4983-tnt-e-assert-false-hangs.test.lua
> @@ -1,70 +1,24 @@
> #!/usr/bin/env tarantool
>
> -local fiber = require('fiber')
> -local clock = require('clock')
> -local ffi = require('ffi')
> -local fio = require('fio')
> -local errno = require('errno')
> +local common = require('process_timeout')
> local tap = require('tap')
> +local fio = require('fio')
> +local ffi = require('ffi')
>
> --
> -- gh-4983: tarantool -e 'assert(false)' hangs
> --
>
> --- For process_is_alive.
> -ffi.cdef([[
> - int kill(pid_t pid, int sig);
> -]])
> -
> ---- Verify whether a process is alive.
> -local function process_is_alive(pid)
> - local rc = ffi.C.kill(pid, 0)
> - return rc == 0 or errno() ~= errno.ESRCH
> -end
> -
> ---- Returns true if process completed before timeout, false otherwise.
> -local function wait_process_completion(pid, timeout)
> - local start_time = clock.monotonic()
> - local process_completed = false
> - while clock.monotonic() - start_time < timeout do
> - if not process_is_alive(pid) then
> - process_completed = true
> - break
> - end
> - fiber.sleep(0.01)
> - end
> - return process_completed
> -end
> -
> ---- Open file on reading with timeout.
> -local function open_with_timeout(filename, timeout)
> - local fh
> - local start_time = clock.monotonic()
> - while not fh and clock.monotonic() - start_time < timeout do
> - fh = fio.open(filename, {'O_RDONLY'})
> - end
> - return fh
> -end
> -
> ---- Try to read from file with timeout with interval.
> -local function read_with_timeout(fh, timeout, interval)
> - local data = ''
> - local start_time = clock.monotonic()
> - while #data == 0 and clock.monotonic() - start_time < timeout do
> - data = fh:read()
> - if #data == 0 then fiber.sleep(interval) end
> - end
> - return data
> -end
> -
> local TARANTOOL_PATH = arg[-1]
> local output_file = fio.abspath('out.txt')
> local line = ('%s -e "assert(false)" > %s 2>&1 & echo $!'):
> format(TARANTOOL_PATH, output_file)
> -local process_waiting_timeout = 30.0
> -local file_read_timeout = 30.0
> -local file_read_interval = 0.2
> -local file_open_timeout = 30.0
> +
> +-- Like a default timeout for `cond_wait` in test-run
> +local process_waiting_timeout = 60.0
> +local file_read_timeout = 60.0
> +local file_open_timeout = 60.0
> +local file_read_interval = 0.01
>
> local res = tap.test('gh-4983-tnt-e-assert-false-hangs', function(test)
> test:plan(2)
> @@ -72,7 +26,7 @@ local res = tap.test('gh-4983-tnt-e-assert-false-hangs', function(test)
> local pid = tonumber(io.popen(line):read('*line'))
> assert(pid, 'pid of proccess can\'t be recieved')
>
> - local process_completed = wait_process_completion(pid,
> + local process_completed = common.wait_process_completion(pid,
> process_waiting_timeout)
>
> local details
> @@ -91,10 +45,10 @@ local res = tap.test('gh-4983-tnt-e-assert-false-hangs', function(test)
> -- Kill process if hangs.
> if not process_completed then ffi.C.kill(pid, 9) end
>
> - local fh = open_with_timeout(output_file, file_open_timeout)
> + local fh = common.open_with_timeout(output_file, file_open_timeout)
> assert(fh, 'error while opening ' .. output_file)
>
> - local data = read_with_timeout(fh, file_read_timeout, file_read_interval)
> + local data = common.read_with_timeout(fh, file_read_timeout, file_read_interval)
> test:like(data, 'assertion failed', 'assertion failure is displayed')
>
> fh:close()
> diff --git a/test/app-tap/lua/process_timeout.lua b/test/app-tap/lua/process_timeout.lua
> new file mode 100644
> index 000000000..141fb0e43
> --- /dev/null
> +++ b/test/app-tap/lua/process_timeout.lua
> @@ -0,0 +1,57 @@
> +local fiber = require('fiber')
> +local clock = require('clock')
> +local ffi = require('ffi')
> +local fio = require('fio')
> +local errno = require('errno')
> +
> +-- For process_is_alive.
> +ffi.cdef([[
> + int kill(pid_t pid, int sig);
> +]])
> +
> +--- Verify whether a process is alive.
> +local function process_is_alive(pid)
> + local rc = ffi.C.kill(pid, 0)
> + return rc == 0 or errno() ~= errno.ESRCH
> +end
> +
> +--- Returns true if process completed before timeout, false otherwise.
> +local function wait_process_completion(pid, timeout)
> + local start_time = clock.monotonic()
> + local process_completed = false
> + while clock.monotonic() - start_time < timeout do
> + if not process_is_alive(pid) then
> + process_completed = true
> + break
> + end
> + end
> + return process_completed
> +end
> +
> +--- Open file on reading with timeout.
> +local function open_with_timeout(filename, timeout)
> + local fh
> + local start_time = clock.monotonic()
> + while not fh and clock.monotonic() - start_time < timeout do
> + fh = fio.open(filename, {'O_RDONLY'})
> + end
> + return fh
> +end
> +
> +--- Try to read from file with timeout with interval.
> +local function read_with_timeout(fh, timeout, interval)
> + local data = ''
> + local start_time = clock.monotonic()
> + while #data == 0 and clock.monotonic() - start_time < timeout do
> + data = fh:read()
> + if #data == 0 then fiber.sleep(interval) end
> + end
> + return data
> +end
> +
> +return {
> + process_is_alive = process_is_alive,
No external function currently uses this function.
I suggest removing it from the interface
> + wait_process_completion = wait_process_completion,
> + open_with_timeout = open_with_timeout,
> + read_with_timeout = read_with_timeout
> +}
> diff --git a/test/app-tap/suite.ini b/test/app-tap/suite.ini
> index 2f16128f9..6e27f88d6 100644
> --- a/test/app-tap/suite.ini
> +++ b/test/app-tap/suite.ini
> @@ -1,7 +1,7 @@
> [default]
> core = app
> description = application server tests (TAP)
> -lua_libs = lua/require_mod.lua lua/serializer_test.lua
> +lua_libs = lua/require_mod.lua lua/serializer_test.lua lua/process_timeout.lua
> is_parallel = True
> pretest_clean = True
> use_unix_sockets_iproto = True
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Tarantool-patches] [PATCH 3/3] test: new version for -e assert(false) test
2020-12-29 16:25 ` [Tarantool-patches] [PATCH 3/3] test: new version for -e assert(false) test Artem Starshov
2020-12-29 17:07 ` Leonid Vasiliev
@ 2020-12-29 20:16 ` Alexander Turenko
1 sibling, 0 replies; 13+ messages in thread
From: Alexander Turenko @ 2020-12-29 20:16 UTC (permalink / raw)
To: Artem Starshov; +Cc: tarantool-patches
On Tue, Dec 29, 2020 at 07:25:35PM +0300, Artem Starshov wrote:
> Functions with timeout transferred to module
> test/app-tap/lua/process_timeout.lua for futher
> using in other tests.
Typo: futher -> further.
>
> Before this version of test was already done,
> but by mistake older version was rebased on
> master.
BTW, I would mark all such commits as 'Follows up #4983'. It is a kind
of grouping and may help to navigate over the history in a future.
> -local fiber = require('fiber')
> -local clock = require('clock')
> -local ffi = require('ffi')
> -local fio = require('fio')
> -local errno = require('errno')
> +local common = require('process_timeout')
Why 'common', not 'process_timeout'?
> diff --git a/test/app-tap/lua/process_timeout.lua b/test/app-tap/lua/process_timeout.lua
> new file mode 100644
> index 000000000..141fb0e43
> --- /dev/null
> +++ b/test/app-tap/lua/process_timeout.lua
I would leave a comment that will describe a reason, why those functions
are necessary in presence of the 'popen' built-in module.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Tarantool-patches] [PATCH 0/3] test: -e assert(false) test fixup
2020-12-29 16:25 [Tarantool-patches] [PATCH 0/3] test: -e assert(false) test fixup Artem Starshov
` (2 preceding siblings ...)
2020-12-29 16:25 ` [Tarantool-patches] [PATCH 3/3] test: new version for -e assert(false) test Artem Starshov
@ 2020-12-29 16:45 ` Artem
2020-12-29 17:11 ` Leonid Vasiliev
4 siblings, 0 replies; 13+ messages in thread
From: Artem @ 2020-12-29 16:45 UTC (permalink / raw)
To: Alexander Turenko; +Cc: tarantool-patches
Branch:
https://github.com/tarantool/tarantool/tree/artemreyt/gh-4983-fix-test
29.12.2020 19:25, Artem Starshov пишет:
> *** BLURB HERE ***
>
> Alexander Turenko (2):
> test: tarantool -e test add more details and avoid busy loop
> github-ci: add init option for containers
>
> Artem Starshov (1):
> test: new version for -e assert(false) test
>
> .github/workflows/coverity.yml | 6 +-
> .github/workflows/debian_11.yml | 1 +
> .github/workflows/debug_coverage.yml | 6 +-
> .github/workflows/luacheck.yml | 6 +-
> .github/workflows/release.yml | 6 +-
> .github/workflows/release_asan_clang11.yml | 6 +-
> .github/workflows/release_clang.yml | 6 +-
> .github/workflows/release_lto.yml | 6 +-
> .github/workflows/release_lto_clang11.yml | 6 +-
> .../gh-4983-tnt-e-assert-false-hangs.test.lua | 81 ++++++-------------
> test/app-tap/lua/process_timeout.lua | 57 +++++++++++++
> test/app-tap/suite.ini | 2 +-
> 12 files changed, 122 insertions(+), 67 deletions(-)
> create mode 100644 test/app-tap/lua/process_timeout.lua
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Tarantool-patches] [PATCH 0/3] test: -e assert(false) test fixup
2020-12-29 16:25 [Tarantool-patches] [PATCH 0/3] test: -e assert(false) test fixup Artem Starshov
` (3 preceding siblings ...)
2020-12-29 16:45 ` [Tarantool-patches] [PATCH 0/3] test: -e assert(false) test fixup Artem
@ 2020-12-29 17:11 ` Leonid Vasiliev
4 siblings, 0 replies; 13+ messages in thread
From: Leonid Vasiliev @ 2020-12-29 17:11 UTC (permalink / raw)
To: Artem Starshov, Alexander Turenko; +Cc: tarantool-patches
Hi! Thank you for the patchset.
On 29.12.2020 19:25, Artem Starshov wrote:
> *** BLURB HERE ***
Garbage.
>
> Alexander Turenko (2):
> test: tarantool -e test add more details and avoid busy loop
> github-ci: add init option for containers
>
> Artem Starshov (1):
> test: new version for -e assert(false) test
Add links to branch and issue.
Also add "Follows up #..." to commit messages.
>
> .github/workflows/coverity.yml | 6 +-
> .github/workflows/debian_11.yml | 1 +
> .github/workflows/debug_coverage.yml | 6 +-
> .github/workflows/luacheck.yml | 6 +-
> .github/workflows/release.yml | 6 +-
> .github/workflows/release_asan_clang11.yml | 6 +-
> .github/workflows/release_clang.yml | 6 +-
> .github/workflows/release_lto.yml | 6 +-
> .github/workflows/release_lto_clang11.yml | 6 +-
> .../gh-4983-tnt-e-assert-false-hangs.test.lua | 81 ++++++-------------
> test/app-tap/lua/process_timeout.lua | 57 +++++++++++++
> test/app-tap/suite.ini | 2 +-
> 12 files changed, 122 insertions(+), 67 deletions(-)
> create mode 100644 test/app-tap/lua/process_timeout.lua
>
^ permalink raw reply [flat|nested] 13+ messages in thread