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

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

>>> Ouch, okay, fill.lua is called on the old tarantool version (on which
>>> you generate a snapshot). It is okay for my case to disable autoupgrade
>>> and insert the data using the new tarantool version. I guess it is okay
>>> too for most of cases. Your case with upgrade of a sequence can be
>>> tested this way too.
>>
>> No, it can't. Unless you tried and actually managed to reproduce this.
>> Because I just tried your way. I made checkout to 2.1, took an empty
>> snapshot (00000000000000000000.snap). Took 2.2 before the sequence fix
>> (that commit: 13de917a0e3b8affb693d041663c174d9ec7fcc9). Then I started
>> Tarantool in read-only mode:
>>
>> <...snipped steps to reproduce...>
>>
>> So even though in your particular case it is ok, it is very
>> artificial, and can't be applied to all tests.
> 
> You're right. I missed that we'll unable to reproduce the problem if
> we'll call fixed upgrade script (from a new tarantool version).
> 
>> Okey, you can omit fill.lua, but I want upgrade-related tests in
>> one place. Or organized in one way in maximum two places, if you are
>> so desperate to use tap.
> 
> I'll rework the patch to keep snapshots together. Don't sure about
> feeding initial data in my case (from fill.lua or from a test): I'll
> took a time to think around.

I am ok with feeding data however you want, suitable for a particular
ticket. But what I consider important is having such tests organized in
one way. With the same folder structure, file naming, isolation.

Although your snapshot is 'pure'. Zero snapshot with no data. Probably
such snapshot could be shared by multiple tests via a symbolic link, if
needed.

> It seems logical to move snapshots out from test suites, since they will
> be used across the test suites. What do you think?

It is not always the case. A snapshot is ok to use across tests only if
it is not related to any particular test more than to others. Only empty
snapshots satisfy that criteria. The reason is that we are obligated to
keep tests isolated from each other. And if you use one non-empty snapshot
for multiple tests, the policy will be violated.

I don't like doing file-per-bug tests, but for snapshots it is fair. Because
otherwise such a snapshot quickly becomes a pile of unmaintainable garbage,
like xlog/upgrade snapshots were beforehand.