[Tarantool-patches] [PATCH v5 1/2] fiber: use diag_ logger in fiber_madvise/mprotect failures
Alexander Turenko
alexander.turenko at tarantool.org
Thu Feb 6 14:10:08 MSK 2020
Everything looks okay expect a couple of minor comments.
WBR, Alexader Turenko.
> fiber: use diag_ logger in fiber_madvise/mprotect failures
We usually try to fit a commit header within 50 symbols (this is however
should not be very strong requirement, IMHO). Say,
| fiber: set diagnostic at mprotect/madvise failure
> diff --git a/test/box/errinj.result b/test/box/errinj.result
> index babe36b1b..c58e1d2e8 100644
> --- a/test/box/errinj.result
> +++ b/test/box/errinj.result
> @@ -1,1783 +1,1887 @@
> +-- test-run result file version 2
Please, don't change format of the result file. See [1], section 'New
result file format'.
In brief, it means to do
$ (cd test && ./test-run.py box/errinj.test.lua && mv box/errinj.{reject,result})
instead of
$ (cd test && rm box/errinj.result && ./test-run.py box/errinj.test.lua)
I was asked to fix it several times and so I will bring the issue [2]
into QA team attention at Friday's status meeting.
[1]: https://github.com/tarantool/test-run/commit/a04b5b096c607172ce4fc86a84e3531c9f3a7304
[2]: https://github.com/tarantool/test-run/issues/194
> diff --git a/test/unit/CMakeLists.txt b/test/unit/CMakeLists.txt
> index 4a57597e9..212a02d36 100644
> --- a/test/unit/CMakeLists.txt
> +++ b/test/unit/CMakeLists.txt
> @@ -74,6 +74,10 @@ add_executable(fiber.test fiber.cc)
> set_source_files_properties(fiber.cc PROPERTIES COMPILE_FLAGS -O0)
> target_link_libraries(fiber.test core unit)
>
> +add_executable(fiber_stack.test fiber_stack.cc)
> +set_source_files_properties(fiber_stack.cc PROPERTIES COMPILE_FLAGS -O0)
> +target_link_libraries(fiber_stack.test core unit)
> +
> if (NOT ENABLE_GCOV)
> # This test is known to be broken with GCOV
> add_executable(guard.test guard.cc)
> diff --git a/test/unit/fiber_stack.cc b/test/unit/fiber_stack.cc
> new file mode 100644
> index 000000000..b402e55d5
> --- /dev/null
> +++ b/test/unit/fiber_stack.cc
> @@ -0,0 +1,83 @@
> +#include "memory.h"
> +#include "fiber.h"
> +#include "unit.h"
> +#include "trivia/util.h"
> +#include "errinj.h"
> +
> +static struct fiber_attr default_attr;
> +
> +static int
> +noop_f(va_list ap)
> +{
> + return 0;
> +}
> +
> +static void
> +fiber_stack_fail_test(void)
> +{
> + struct errinj *inj;
> + struct fiber *fiber;
> +
> + header();
> +
> + /*
> + * Set non-default stack size to prevent reusing of an
> + * existing fiber.
> + */
Space/tab indent mix.
> + struct fiber_attr *fiber_attr = fiber_attr_new();
> + fiber_attr_setstacksize(fiber_attr, default_attr.stack_size * 2);
> +
> + /*
> + * Clear the fiber's diagnostics area to check that failed
> + * fiber_new() sets an error.
> + */
Same.
> diff --git a/test/unit/fiber_stack.result b/test/unit/fiber_stack.result
> new file mode 100644
> index 000000000..af10ab4f1
> --- /dev/null
> +++ b/test/unit/fiber_stack.result
> @@ -0,0 +1,6 @@
> + *** fiber_stack_fail_test ***
> +ok 1 - mprotect: failed to setup fiber guard page
> +ok 2 - mprotect: diag is armed after error
> +ok 3 - madvise: non critical error on madvise hint
> +ok 4 - madvise: diag is armed after error
> + *** fiber_stack_fail_test: done ***
It fails like so for me:
[001] Test failed! Result content mismatch:
[001] --- unit/fiber_stack.result Thu Feb 6 10:36:02 2020
[001] +++ unit/fiber_stack.reject Thu Feb 6 13:29:15 2020
[001] @@ -1,3 +1,4 @@
[001] +SystemError fiber mprotect failed: Cannot allocate memory
[001] *** fiber_stack_fail_test ***
[001] ok 1 - mprotect: failed to setup fiber guard page
[001] ok 2 - mprotect: diag is armed after error
It seems test-run mixes stdout and stderr for unit tests.
There is the common problem: 'release_disabled' tests are not run at all
for 'core = app' and 'core = unittest' test suites. I filed [3] for
this. It means for now that you should verify your test locally w/o
release_disabled, but then put it to release_disabled back before push.
[3]: https://github.com/tarantool/test-run/issues/199
More information about the Tarantool-patches
mailing list