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

[Alexander Turenko <alexander.turenko@tarantool.org>]

On Sat, Apr 04, 2020 at 12:53:53AM +0200, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
> 
> On 01/04/2020 22:45, Alexander Turenko wrote:
> > After 2.2.0-633-gaa0964ae1 ('net.box: fix schema fetching from 1.10/2.1
> > servers') net.box expects that _vcollation system view exists on a
> > tarantool server of 2.2.1+ version. This is however not always so: a
> > server may be run on a new version of tarantool, but work on a schema of
> > an old version.
> > 
> > The situation with non last schema is usual for replication cluster in
> > process of upgrading: all instances run on the new version of tarantool
> > first (no auto-upgrade is performed by tarantools in a cluster). Then
> > box.schema.upgrade() should be called, but the instances should be
> > operable even before the call.
> > 
> > Before the commit net.box was unable to connect a server if it is run on
> > a schema without _vcollation system view (say, 2.1.3), but the server
> > executable is of 2.2.1 version or newer.
> > 
> > Follows up #4307
> > Fixes #4691
> > ---
> > 
> > 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.

Not exactly.

I want to test the case when an instance is NOT upgraded. So 'upgrade'
directory name for snaps would be misleading.

I also think that we should move from using compare-against-output
tests. 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).

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.

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). Sometimes it eases
reading of test cases, especially when you're not much familiar with a
testing system. I don't much like the approach with fill.lua script (at
least for data corresponds to test cases).

WBR, Alexander Turenko.