From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 150B44696C3 for ; Sat, 4 Apr 2020 20:40:46 +0300 (MSK) References: <65798b71932c21183072decd2cce8b2b0f88b884.1585773275.git.alexander.turenko@tarantool.org> <78056845-c72f-f97e-de62-2f04baaed0e0@tarantool.org> <20200403233859.57lre4ef6co22g3e@tkn_work_nb> From: Vladislav Shpilevoy Message-ID: <61a72f34-7bd0-576e-e305-dfee085ca724@tarantool.org> Date: Sat, 4 Apr 2020 19:40:44 +0200 MIME-Version: 1.0 In-Reply-To: <20200403233859.57lre4ef6co22g3e@tkn_work_nb> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH] net.box: fix fetching of schema of an old version List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Turenko Cc: tarantool-patches@dev.tarantool.org >>> https://github.com/tarantool/tarantool/issues/4691 >>> https://github.com/tarantool/tarantool/tree/Totktonada/gh-4691-net-box-connect-schema-2-1-3 >>> >>> src/box/lua/net_box.lua | 9 ++ >>> ...4691-net-box-connect-schema-2-1-3.test.lua | 93 ++++++++++++++++++ >>> test/box-tap/no_auto_schema_upgrade.lua | 30 ++++++ >>> .../snap/2.1.3/00000000000000000000.snap | Bin 0 -> 4466 bytes >>> 4 files changed, 132 insertions(+) >>> create mode 100755 test/box-tap/gh-4691-net-box-connect-schema-2-1-3.test.lua >> >> We have xlog/upgrade suite specifically for these tests. With >> already designed processes, where and how to store old snaps, etc. >> See xlog/upgrade/how_to_add_new_test.md. > > > I want to test the case when an instance is NOT upgraded. So 'upgrade' > directory name for snaps would be misleading. The test and the bug are still upgrade related. 'xlog/upgrade' is not called 'xlog/upgrade_called'. It is called just 'upgrade'. For all cases when an old snapshot is used to start a new tarantool. > I also think that we should move from using compare-against-output > tests. This is not about tap vs diff. It is about having all upgrade related tests in one place, and organized in one way, with how to fill them, with how to store xlogs and snapshots in the repository. > I'll not insist on this during review, but personally I will > create new tests using pytest-like (phpunit-like, maven-failsafe-like, > luatest-like) way (it is our TAP13 tests or at least near to them). I > can summarize cons and pros if you want (don't sure whether I did it > already somewhere). Don't really want to discuss this again, everything was said already 1000 times or more. > We can move 'snap' directory upward (right into test/) and use it for > snapshots for all test suites. Or keep them per-suite. However see the > next paragraph re fill.lua. This is a bad idea, because different upgrade-related tests usually need some specific data stored in the old snapshot. If you merge data, needed by many tests, into one snapshot, you will violate the policy of one-bug-one-test. Because many tests will start to depend on each other. > I prefer to feed initial schema and data right from a test to keep them > near to actual usage in test cases (within one file). This makes impossible to test bugs, which appear only when you bootstrap from the old snapshot. > Sometimes it eases > reading of test cases, especially when you're not much familiar with a > testing system. You are now creating a second way of testing upgrade. This is definitely not a simplification of the testing system. > I don't much like the approach with fill.lua script (at > least for data corresponds to test cases). fill.lua is inevitable, when you need to test bootstrap from an old snapshot having some data in it before you start tarantool, and you want to know what data is stored here exactly.