Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

app+luatest: eliminate stdout buffering and stream mixing problems #394

Merged
merged 8 commits into from
May 31, 2023

Conversation

Totktonada
Copy link
Member

The patchset attempts to eliminate unnecessary bufferization and stdout/stderr streams mixing. It should improve observability in case of a failure (#119) and stability for tests that intensively writes to stderr (#392).

  • app+luatest: add -e "io.stdout:setvbuf('no')" to tarantool command-line arguments.
  • app+luatest: don't collect stdout in the memory till process termination, but write it directly to the temporary result file.
  • luatest: split stdout and stderr files to give the TAP13 parser more chances to understand the stdout correctly.

Fixes #119
Fixes #392


I test it on small examples like the following:

$ cat test/app-tap/foo.test.lua
#!/usr/bin/env tarantool

local tap = require('tap')

local test = tap.test('foo')

while true do
    test:ok(true, 'foo foo foo')
    io.stderr:write('bar bar bar\n')
    require('fiber').sleep(1)
end
$ cat test/app-luatest/foo_test.lua 
local t = require('luatest')
local g = t.group()

g.test_foo = function()
    while true do
        t.assert_equals(1, 1)
        io.stderr:write('bar bar bar\n')
        require('fiber').sleep(1)
    end
end

And I didn't verify it on the real world tests.

So, please, accept the patches with caution.

There is no sense to wait for the process termination or timeout.

It allows to see the output as the file's content during a test
execution.
Quote from luatest/runner.lua:

> -c  Disable capture
Parse stdout as TAP13. Write stderr to a log file to show on a test
failure.

Fixes #392
There is no sense to wait till process termination and accumulate all
the output in the memory.
LuatestServer.find_exe() already adds this property to the class.
@Totktonada Totktonada force-pushed the gh-119-buffering-problems branch from 9b5199d to 7c10720 Compare May 30, 2023 12:44
The ideas behind the change:

* The log is sometimes from a tarantool instance, but a luatest based
  test may print logs from another process to stderr. Use more general
  term 'log' to don't confuse anyone.
* Replace term 'instance' with 'test-run server' due to the same
  reasons.
* Make zero size and no file situations more explicit.
* Catch and report 'no logfile property' situation (shouldn't occur, but
  was semi-handled by this code previously for some reason).
* Reduce code reusability for the sake of readability and to ease future
  modifications.
@Totktonada Totktonada requested a review from ylobankov May 30, 2023 14:48
@ylobankov ylobankov merged commit 7a1ec4f into master May 31, 2023
@ylobankov ylobankov deleted the gh-119-buffering-problems branch May 31, 2023 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

luatest: mixing of stdout and stderr has bad consequences A hang core = app test does not show an output
2 participants