Re: [Tarantool-patches] [PATCH] net.box: fix fetching of schema of an old version

[Vladislav Shpilevoy <v.shpilevoy@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.