From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id BF3492B613 for ; Sun, 15 Apr 2018 09:18:10 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id eMC1_034i0As for ; Sun, 15 Apr 2018 09:18:10 -0400 (EDT) Received: from smtp15.mail.ru (smtp15.mail.ru [94.100.176.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 11FBA2B4B2 for ; Sun, 15 Apr 2018 09:18:09 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v2 2/2] Fixed lost format on update and upsert operations. References: <1f4e0a5e-30f2-f36a-3970-ec4eeb14a456@tarantool.org> <4a4c5106-1d2d-553f-c8f1-5c031e91c9ac@tarantool.org> From: Vladislav Shpilevoy Message-ID: Date: Sun, 15 Apr 2018 16:18:06 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org, Kirill Shcherbatov Hello. See below 6 comments. 1. I fixed some of my comments, and pushed them with --force. You can see them using diff your local and remote changes. After this do pull. Please, try to investigate, why your text editor uses spaces for multiline statements alignment, and fix it. 2. I do not see a test on upsert - please, add. 3. Please, add a test on incorrect update. 4. In the previous letter I have asked you to move the test into engine/ suite. You can not get different engines on box/ suite: [001] Test failed! Result content mismatch: [001] --- box/update.result Sun Apr 15 15:55:23 2018 [001] +++ box/update.reject Sun Apr 15 16:07:40 2018 [001] @@ -861,6 +861,10 @@ [001] engine = inspector:get_cfg('engine') [001] --- [001] ... [001] +engine [001] +--- [001] +- null [001] +... [001] format = {} [001] --- [001] ... [001] As you can see, I printed engine variable in you test and 1) it is null, 2) the test is not run 2 times - with engine = memtx and engine = vinyl. For this you must use engine/ suite, not box/. > +test_run = require('test_run') > +inspector = test_run.new() 5. It is not needed to create a new test run each time when you need to get a variable from environment. Here the test run already is created. > +engine = inspector:get_cfg('engine') > format = {} > -s = box.schema.space.create('tst_sample', {engine = 'vinyl', format = format} > -pk = s:create_index('pk') > -s:insert({1, 'sss'}) > -aa = box.space.tst_sample:get(1) > -aa.VAL > -aa = aa:update({{'=',2,'ssss'}}) > -collectgarbage() > -aa.VAL > -s:drop() > -aa = nil > -collectgarbage() > > -- test transform integrity > space = box.schema.space.create('tweedledum') 6. Try to do not create a new space to test each tuple. You can create one space, and test on it transform, correct update, correct upsert, incorrect update, incorrect upsert. Too many actions for such simple test is not good, because test count grows every day, and running all of them is already too long.