From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 08E916E231; Wed, 11 Nov 2026 18:24:51 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 08E916E231 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1794410691; bh=24GWhShsIXjH/tOfLalFuEOye0004boU9b8bn6PKxRY=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=BKoCPKUsZsFR1uB+YzW8/TtcWmRNQkxuSmP+yYa88n4jMY+9LgW+/O3uCfT8L9stb arEtpmigF70poS4nyMTqSiXmHtfIMAOxAgNYK4RHPg/NqLWm3kLmNAZ+4I+MaEyJqc blqEs/d5S5Sy5Oj4gwQCvIe6om0u86uMu7T9Nmdc= Received: from send150.i.mail.ru (send150.i.mail.ru [89.221.237.245]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 4077F6E231 for ; Wed, 11 Nov 2026 18:24:50 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 4077F6E231 Received: by exim-smtp-canary-30-56f4b486d4-pc58m with esmtpa (envelope-from ) id 1xRABs-00000000HQD-2G2R; Wed, 11 Nov 2026 18:24:49 +0300 Message-ID: Date: Wed, 11 Nov 2026 18:24:48 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Sergey Bronnikov References: <4dd8dd2aecca35784e687f7920e6880ad0211f3c.1793624427.git.estetus@gmail.com> Content-Language: en-US In-Reply-To: <4dd8dd2aecca35784e687f7920e6880ad0211f3c.1793624427.git.estetus@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9CC39C64ECE24C2D7142C69BBFDA6F654EA84A054FDCD7DE1182A05F538085040B0A37206C7DCD5453DE06ABAFEAF67053E28F67BCD1D85C2E75D37D82E6BAF12B3FF27712106A5C5 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE70312E9A300D47E3BEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637FF37B1FF54E2C7C48638F802B75D45FF914D58D5BE9E6BC1A93B80C6DEB9DEE97C6FB206A91F05B2B0D5978A93C7974A2E070BE324C7D3C4BF8F75E66DAD0D7CF6B57BC7E64490611E7FA7ABCAF51C92176DF2183F8FC7C0C26CFBAC0749D213D2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A127C277FBC8AE2E8B18E7440D383AA13B3AA81AA40904B5D99C9F4D5AE37F343AD1F44FA8B9022EA23BBE47FD9DD3FB595F5C1EE8F4F765FC72CEEB2601E22B093A03B725D353964B0B7D0EA88DDEDAC722CA9DD8327EE493B89ED3C7A628178135872C767BF85DA2F004C90652538430E4A6367B16DE6309 X-C1DE0DAB: 0D63561A33F958A561FB8EA878DB11DB5002B1117B3ED69650771C41A820EB52BFF4097FFC9E796F823CB91A9FED034534781492E4B8EEADB05233B9BC4759D3 X-C8649E89: 1C3962B70DF3F0AD73CAD6646DEDE191716CD42B3DD1D34CAB70F9BE574AE9C625B6776AC983F447FC0B9F89525902EE6F57B2FD27647F25E66C117BDB76D659EB332EABDE600BB2419AE4B8164A24BE34293CB038BA17BD70D026B6A81B42B7DE5FD47F365FDF35B8341EE9D5BE9A0A5A6171020C0A14CD47E5CC68B88427418937CBD1C17C1F7F9350E1BD188BD56E4C41F94D744909CEEF9E35A692215F79327F81E6EEE0AEB67A0F9F91F029B6AF33F2C28C22F508233FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu53w8ahmwBjZKM/YPHZyZHvz5uv+WouB9+ObcCpyrx6l7KImUglyhkEat/+ysWwi0gdhEs0JGjl6ggRWTy1haxBpVdbIX1nthFXOcIETfglQORZ0zpDET4Zrk3igikrdHlWOr4Rw8NrBY/OphDQ8QPJMA= X-Mailru-Sender: 015BA6FC0EFA223A63A6570A1A03128500AAD96CD535D919B951B70A5BD4BD8EFDC9CEAF1DE3EE4E7B57453E6972826AE62B12C1FE0372A178274A4A9E9E44FD2A114EDA040D8818F01FF9DF31C397A372D6B4FCE48DF648AE208404248635DF X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 1/3] test: add unit tests for the write buffer X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Evgeniy Temirgaleev via Tarantool-patches Reply-To: Evgeniy Temirgaleev Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Sergey! Thanks for the patch! LGTM with some comments. On 11/2/26 4:12 PM, Sergey Bronnikov wrote: > From: Sergey Bronnikov > > The write buffer accumulates the profilers' output and passes it > to the writer on flush. It has no unit tests, so cover writing of > the data of all kinds, flushing of a full buffer, the explicit > flush and the error reporting. > > Part of tarantool/tarantool#11620 > --- > test/tarantool-c-tests/unit-wbuf.test.c | 388 ++++++++++++++++++++++++ > 1 file changed, 388 insertions(+) > create mode 100644 test/tarantool-c-tests/unit-wbuf.test.c > > diff --git a/test/tarantool-c-tests/unit-wbuf.test.c b/test/tarantool-c-tests/unit-wbuf.test.c > new file mode 100644 > index 00000000..4bee6243 > --- /dev/null > +++ b/test/tarantool-c-tests/unit-wbuf.test.c > @@ -0,0 +1,388 @@ > +/* > +** Testing of the write buffer. > +** > +** The write buffer accumulates the profilers' output and passes > +** it to the writer on flush. The tests below cover writing of the > +** data of all kinds, flushing on overflow, the explicit flush and > +** the error reporting. > +*/ > + > +#include > + > +#include > +#include > +#include > + > +#include "lj_wbuf.h" > +#include "test.h" > + > +/* Size of the write buffer used in the tests. */ > +#define BUFFER_SIZE 10 > + > +/* Size of the payload that does not fit into the write buffer. */ > +#define PAYLOAD_SIZE (2 * BUFFER_SIZE + 5) > + > +/* What the writer does on flush. */ > +enum writer_mode { > + /* Report all the data as written. */ > + WMODE_OK, > + /* Report a half of the data as written and keep the buffer. */ > + WMODE_PARTIAL, > + /* Imitate an unrecoverable write error, e.g. ENOSPC. */ > + WMODE_FAIL, > +}; > + > +/* Writer context for simulating different write outcomes. */ > +struct writer_ctx { > + /* What the writer does on flush. */ > + enum writer_mode mode; > + /* Number of performed flushes. */ > + size_t flushes; > + /* Number of bytes reported as written. */ > + size_t written; > + /* Everything reported as written, in order of writing. */ > + uint8_t out[2 * PAYLOAD_SIZE]; > +}; > + > +static size_t test_writer(const void **buf_addr, size_t len, void *opt) > +{ > + struct writer_ctx *ctx = opt; > + size_t n = len; > + > + ctx->flushes++; > + if (ctx->mode == WMODE_FAIL) { > + errno = ENOSPC; > + *buf_addr = NULL; > + return 0; > + } > + if (ctx->mode == WMODE_PARTIAL) { > + errno = EIO; > + n = len / 2; > + } Please, add an assertion to check "ctx->out[] have room for write" and ctx->mode == WMODE_OK (or use switch-case with halt on default branch) to handle possible tests misconfig. > + memcpy(ctx->out + ctx->written, *buf_addr, n); > + ctx->written += n; > + return n; > +} > + > +static void writer_ctx_init(struct writer_ctx *ctx, enum writer_mode mode) > +{ > + memset(ctx, 0, sizeof(*ctx)); > + ctx->mode = mode; > +} > + > +static size_t buf_len(const struct lj_wbuf *buf) > +{ > + return (size_t)(buf->pos - buf->buf); > +} > + Please, rename to wbuf_len as in lj_wbuf.c. May be add a comment also: a copy from... > +static void assert_stream_stopped_with_error(const struct lj_wbuf *buf) > +{ > + assert_true(lj_wbuf_test_flag(buf, STREAM_STOP)); > + assert_true(lj_wbuf_test_flag(buf, STREAM_ERRIO)); > + assert_int_equal(lj_wbuf_errno(buf), ENOSPC); > + assert_ptr_equal(buf->buf, NULL); > + assert_ptr_equal(buf->pos, NULL); > +} > + > +static int test_addbyte(void *state) > +{ > + uint8_t mem[BUFFER_SIZE]; > + struct lj_wbuf buf; > + struct writer_ctx ctx; > + > + UNUSED(state); > + > + writer_ctx_init(&ctx, WMODE_OK); > + lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem)); > + > + lj_wbuf_addbyte(&buf, 0x7f); > + assert_sizet_equal(buf_len(&buf), 1); > + assert_int_equal(mem[0], 0x7f); > + assert_sizet_equal(ctx.flushes, 0); > + assert_int_equal(lj_wbuf_errno(&buf), 0); > + > + return TEST_EXIT_SUCCESS; > +} > + > +static int test_addu64(void *state) > +{ > + uint8_t mem[BUFFER_SIZE]; > + struct lj_wbuf buf; > + struct writer_ctx ctx; > + > + UNUSED(state); > + > + writer_ctx_init(&ctx, WMODE_OK); > + lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem)); > + > + /* 128 is encoded as two bytes. */ Please, add a check assert(2 < BUFFER_SIZE) to comment why ctx.flushes must be zero. > + lj_wbuf_addu64(&buf, 128); > + assert_sizet_equal(buf_len(&buf), 2); > + assert_int_equal(mem[0], 0x80); > + assert_int_equal(mem[1], 0x01); > + assert_sizet_equal(ctx.flushes, 0); > + > + return TEST_EXIT_SUCCESS; > +} > + > +static int test_addn(void *state) > +{ > + uint8_t mem[BUFFER_SIZE]; > + struct lj_wbuf buf; > + struct writer_ctx ctx; > + > + UNUSED(state); > + > + writer_ctx_init(&ctx, WMODE_OK); > + lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem)); > + assert(3 < BUFFER_SIZE); > + lj_wbuf_addn(&buf, "xyz", 3); > + assert_sizet_equal(buf_len(&buf), 3); > + assert_true(memcmp(mem, "xyz", 3) == 0); > + assert_sizet_equal(ctx.flushes, 0); > + > + return TEST_EXIT_SUCCESS; > +} > + > +static int test_addstring(void *state) > +{ > + uint8_t mem[BUFFER_SIZE]; > + struct lj_wbuf buf; > + struct writer_ctx ctx; > + > + UNUSED(state); > + > + writer_ctx_init(&ctx, WMODE_OK); > + lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem)); > + > + /* A string is a ULEB128 length followed by the payload. */ assert(4 < BUFFER_SIZE); > + lj_wbuf_addstring(&buf, "xyz"); > + assert_sizet_equal(buf_len(&buf), 4); > + assert_int_equal(mem[0], 3); > + assert_true(memcmp(mem + 1, "xyz", 3) == 0); > + assert_sizet_equal(ctx.flushes, 0); > + > + return TEST_EXIT_SUCCESS; > +} > + > +static int test_addn_pumped_through_buffer(void *state) > +{ > + uint8_t mem[BUFFER_SIZE]; > + uint8_t payload[PAYLOAD_SIZE]; > + struct lj_wbuf buf; > + struct writer_ctx ctx; > + > + UNUSED(state); > + > + memset(payload, 'a', sizeof(payload)); > + writer_ctx_init(&ctx, WMODE_OK); > + lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem)); > + > + /* > + * The payload does not fit into the buffer, so it is > + * pumped through it. Two full buffers are flushed, the > + * rest stays in the buffer. > + */ > + lj_wbuf_addn(&buf, payload, sizeof(payload)); > + assert_sizet_equal(ctx.flushes, 2); > + assert_sizet_equal(ctx.written, 2 * BUFFER_SIZE); > + assert_true(memcmp(ctx.out, payload, 2 * BUFFER_SIZE) == 0); > + assert_sizet_equal(buf_len(&buf), PAYLOAD_SIZE - 2 * BUFFER_SIZE); > + assert_true(memcmp(mem, payload + 2 * BUFFER_SIZE, > + PAYLOAD_SIZE - 2 * BUFFER_SIZE) == 0); > + > + return TEST_EXIT_SUCCESS; > +} A check for 'no error case' (implicit flush) after pumping as in the flush test is needed. > + > +static int test_flush_on_overflow(void *state) > +{ > + uint8_t mem[BUFFER_SIZE]; > + uint8_t payload[BUFFER_SIZE]; > + struct lj_wbuf buf; > + struct writer_ctx ctx; > + > + UNUSED(state); > + > + memset(payload, 'a', sizeof(payload)); > + writer_ctx_init(&ctx, WMODE_OK); > + lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem)); Nit, may be add a macro with common header for all tests to reduce text size and to simplify reading? Something like: #define TEST_INIT(mode) uint8_t mem[BUFFER_SIZE]; \ struct lj_wbuf buf; \ struct writer_ctx ctx; \ writer_ctx_init(&ctx, (mode)); \ lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem)); > + > + /* A byte written to a full buffer is preceded by a flush. */ > + lj_wbuf_addn(&buf, payload, sizeof(payload)); > + assert_sizet_equal(buf_len(&buf), sizeof(mem)); > + lj_wbuf_addbyte(&buf, 0xaa); > + assert_sizet_equal(ctx.flushes, 1); > + assert_sizet_equal(ctx.written, sizeof(mem)); > + assert_sizet_equal(buf_len(&buf), 1); > + assert_int_equal(mem[0], 0xaa); > + > + /* The same for an ULEB128 value written to a full buffer. */ > + lj_wbuf_addn(&buf, payload, sizeof(payload) - 1); > + assert_sizet_equal(buf_len(&buf), sizeof(mem)); > + lj_wbuf_addu64(&buf, 0); > + assert_sizet_equal(ctx.flushes, 2); > + assert_sizet_equal(ctx.written, 2 * sizeof(mem)); > + assert_sizet_equal(buf_len(&buf), 1); > + > + /* And for a data block written to a full buffer. */ > + lj_wbuf_addn(&buf, payload, sizeof(payload) - 1); > + assert_sizet_equal(buf_len(&buf), sizeof(mem)); > + lj_wbuf_addn(&buf, payload, 1); > + assert_sizet_equal(ctx.flushes, 3); > + assert_sizet_equal(ctx.written, 3 * sizeof(mem)); > + assert_sizet_equal(buf_len(&buf), 1); > + > + return TEST_EXIT_SUCCESS; > +} > + > +static int test_flush(void *state) > +{ > + uint8_t mem[BUFFER_SIZE]; > + struct lj_wbuf buf; > + struct writer_ctx ctx; > + > + UNUSED(state); > + > + writer_ctx_init(&ctx, WMODE_OK); > + lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem)); > + > + lj_wbuf_addn(&buf, "xyz", 3); > + lj_wbuf_flush(&buf); > + assert_sizet_equal(ctx.flushes, 1); > + assert_sizet_equal(ctx.written, 3); > + assert_true(memcmp(ctx.out, "xyz", 3) == 0); > + /* The buffer is empty after a successful flush. */ > + assert_sizet_equal(buf_len(&buf), 0); > + assert_false(lj_wbuf_test_flag(&buf, STREAM_ERRIO | STREAM_STOP)); > + > + /* The buffer is usable after the flush. */ > + lj_wbuf_addbyte(&buf, 0x7f); > + assert_sizet_equal(buf_len(&buf), 1); > + assert_int_equal(mem[0], 0x7f); > + > + return TEST_EXIT_SUCCESS; > +} > + > +static int test_flush_partial_write(void *state) > +{ > + uint8_t mem[BUFFER_SIZE]; > + struct lj_wbuf buf; > + struct writer_ctx ctx; > + > + UNUSED(state); > + > + writer_ctx_init(&ctx, WMODE_PARTIAL); > + lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem)); > + > + lj_wbuf_addn(&buf, "xyz", 3); > + lj_wbuf_flush(&buf); > + /* A partial write is reported as an error. */ > + assert_true(lj_wbuf_test_flag(&buf, STREAM_ERRIO)); > + assert_int_equal(lj_wbuf_errno(&buf), EIO); > + /* The writer keeps the buffer, so the stream is not stopped. */ > + assert_false(lj_wbuf_test_flag(&buf, STREAM_STOP)); > + assert_ptr_equal(buf.buf, mem); > + /* The unwritten data is dropped. */ > + assert_sizet_equal(buf_len(&buf), 0); > + > + return TEST_EXIT_SUCCESS; > +} > + > +static int test_flush_stopped_stream(void *state) > +{ > + uint8_t mem[BUFFER_SIZE]; > + struct lj_wbuf buf; > + struct writer_ctx ctx; > + > + UNUSED(state); > + > + writer_ctx_init(&ctx, WMODE_FAIL); > + lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem)); > + > + lj_wbuf_addn(&buf, "xyz", 3); > + lj_wbuf_flush(&buf); > + assert_stream_stopped_with_error(&buf); > + assert_sizet_equal(ctx.flushes, 1); > + > + /* A stopped stream is not flushed again. */ > + lj_wbuf_flush(&buf); > + assert_sizet_equal(ctx.flushes, 1); > + > + return TEST_EXIT_SUCCESS; > +} > + > +static int test_write_to_stopped_stream(void *state) > +{ > + uint8_t mem[BUFFER_SIZE]; > + struct lj_wbuf buf; > + struct writer_ctx ctx; > + > + UNUSED(state); > + > + writer_ctx_init(&ctx, WMODE_FAIL); > + lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem)); > + > + lj_wbuf_addn(&buf, "xyz", 3); > + lj_wbuf_flush(&buf); > + assert_stream_stopped_with_error(&buf); > + > + /* Writes to a stopped stream are ignored. */ > + lj_wbuf_addbyte(&buf, 0x7f); > + lj_wbuf_addu64(&buf, 128); > + lj_wbuf_addn(&buf, "xyz", 3); > + lj_wbuf_addstring(&buf, "xyz"); > + assert_stream_stopped_with_error(&buf); > + assert_sizet_equal(ctx.flushes, 1); > + assert_sizet_equal(ctx.written, 0); > + > + return TEST_EXIT_SUCCESS; > +} > + > +static int test_terminate(void *state) > +{ > + uint8_t mem[BUFFER_SIZE]; > + struct lj_wbuf buf; > + struct writer_ctx ctx; > + > + UNUSED(state); > + > + writer_ctx_init(&ctx, WMODE_OK); > + lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem)); > + > + lj_wbuf_terminate(&buf); > + assert_true(buf.writer == NULL); > + assert_ptr_equal(buf.ctx, NULL); > + assert_ptr_equal(buf.buf, NULL); > + assert_ptr_equal(buf.pos, NULL); > + assert_sizet_equal(buf.size, 0); > + assert_false(lj_wbuf_test_flag(&buf, STREAM_ERRIO | STREAM_STOP)); > + assert_int_equal(lj_wbuf_errno(&buf), 0); > + > + return TEST_EXIT_SUCCESS; > +} > + > +/* > +** A writer may release the buffer and set *data to NULL on an > +** unrecoverable error (e.g. ENOSPC). The stream is stopped in > +** this case, so the write functions must not touch the released > +** buffer. > +*/ It seems some code was omitted or added later? This functions: static int test_addbyte_failed_flush(void *state) static int test_addu64_failed_flush(void *state) static int test_addn_failed_flush_in_loop(void *state) -- A note about pumped write (addn) implementation: the pumping loop doesn't stops if an IO error occurs, it may lose some content inside src[n] as the flush discards unwritten data in some iterations. E.g. if we write to socket and errno=EAGAIN occurs, there written < len is valid case. It looks strange: what is the purpose of this behavior? The error in any iteration corrupts the whole write. Client must check against error flag always and terminate or repeat. (The LuaJIT code checks against STREAM_ERRIO|STREAM_STOP always.) Its a questinon: Why the loop is not terminated as it continue with an error is useless? I think it will be good to somehow put this information into the test suite: make a test case with WMODE_PARTIAL for pumping addn() with corresponding checks (have an error flag only and have not all data written). > +int main(void) > +{ > + const struct test_unit tgroup[] = { > + test_unit_def(test_addbyte), > + test_unit_def(test_addu64), > + test_unit_def(test_addn), > + test_unit_def(test_addstring), > + test_unit_def(test_addn_pumped_through_buffer), > + test_unit_def(test_flush_on_overflow), > + test_unit_def(test_flush), > + test_unit_def(test_flush_partial_write), > + test_unit_def(test_flush_stopped_stream), > + test_unit_def(test_write_to_stopped_stream), > + test_unit_def(test_terminate), > + }; > + const int test_result = test_run_group(tgroup, NULL); > + > + return test_result; > +} -- -- Best regards, Evgeniy Temirgaleev