Commit aa7e1966 authored by Jérome Perrin's avatar Jérome Perrin Committed by Kirill Smelkov

zodbrestore: Mark restore-with-extension tests as xfail on ZODB4

@kirr wrote (!19 (comment 129442))

    For the reference - contrary to ZODB5, restore tests on ZODB4 are currently
    [broken](https://nexedijs.erp5.net/#/test_result_module/20210317-B3AC205A/2).
    Restored file is not bit-to-bit identical to the original.

    The problem is that on commit/restore, we need to save
    user/description/extension. For extension `zodbdump.Transaction` provides
    .extension_bytes, which ZODB5 uses to save its raw copy. However ZODB4 goes
    through `.extension` and pickles it:

    https://lab.nexedi.com/nexedi/zodbtools/blob/129afa67/zodbtools/zodbdump.py#L425-453
    https://github.com/zopefoundation/ZODB/blob/4/src/ZODB/BaseStorage.py#L220-L240

    This leads to unpickle-repickle round-trip and different extension being committed on restore:

    ```diff
    diff --git a/1zdump b/2zdump
    index 5033bc1..a3a32aa 100644
    --- a/1zdump
    +++ b/2zdump
    @@ -10,7 +10,7 @@ q^A.
     txn 0285cbac3d0369e6 " "
     user "user0.0"
     description "step 0.0"
    -extension "\x80\x02}q\x01(U\tx-cookieSU\x05RF9IEU\vx-generatorq\x02U\fzodb/py2 (f)u."
    +extension "}q\x01(U\tx-cookieSU\x05RF9IEU\vx-generatorU\fzodb/py2 (f)u."
     obj 0000000000000000 98 sha1:eba252d1984f975ecb636bc1b3a89c953dd20527
    ...
    ```

    What might save us is to somehow in Transaction.extension returns a
    dict-subclass object that is somehow pickled to the exact bytes remembered when
    it was created. However, after briefly checking, I could not find a mechanism
    to do so yet...

@jerome wrote (!19 (comment 129479))

    @kirr we already have pytest fixtures to test differently depending on whether
    the ZODB version has support for extension_bytes, so what about using it in the
    test and testing restoring the extension bytes version of the dump only for
    ZODB5 ?

@kirr wrote (!19 (comment 129482))

    @jerome, yes we have this, but I believe we should actually fix zodbrestore to
    be reliable whatever ZODB is used. For ZODB5 it works. For ZODB4-wc2 we can
    adjust ZODB code to use extension_bytes similarly to how ZODB5 does. But
    unpatched ZODB4 is currently out of luck. As it was decided that Nexedi will
    use both ZODB4 and ZODB4-wc2, I think we should fix zodbrestore to work on all
    those versions to be reliable.

    /cc @tomo

@kirr:

-> No universal ZODB4 fix for now (this would require to monkey patch ZODB in
several places), so mark "restore with extension" test as xfail similarly to
how we already do for "dump with extension" test.

This brings -ZODB4 and -ZODB4-wc2 tests back to PASS state.

Even though on ZODB4 extension is restored not bit-to-bit exactly, it is
restored to be the same dictionary equal to what was used to produce the
dump. Not ideal, but still not loosing the information in practice.

One more reason to switch to ZODB5...
parent 129afa67
...@@ -30,16 +30,17 @@ from golang import func, defer ...@@ -30,16 +30,17 @@ from golang import func, defer
# verify zodbrestore. # verify zodbrestore.
@func @func
def test_zodbrestore(): def test_zodbrestore(zext):
tmpd = mkdtemp('', 'zodbrestore.') tmpd = mkdtemp('', 'zodbrestore.')
defer(lambda: rmtree(tmpd)) defer(lambda: rmtree(tmpd))
zkind = '_!zext' if zext.disabled else ''
# restore from testdata/1.zdump.ok and verify it gives result that is # restore from testdata/1.zdump.ok and verify it gives result that is
# bit-to-bit identical to testdata/1.fs # bit-to-bit identical to testdata/1.fs
tdata = dirname(__file__) + "/testdata" tdata = dirname(__file__) + "/testdata"
@func @func
def _(): def _():
zdump = open("%s/1.zdump.ok" % tdata, 'rb') zdump = open("%s/1%s.zdump.ok" % (tdata, zkind), 'rb')
defer(zdump.close) defer(zdump.close)
stor = storageFromURL('%s/2.fs' % tmpd) stor = storageFromURL('%s/2.fs' % tmpd)
...@@ -48,7 +49,7 @@ def test_zodbrestore(): ...@@ -48,7 +49,7 @@ def test_zodbrestore():
zodbrestore(stor, zdump) zodbrestore(stor, zdump)
_() _()
zfs1 = _readfile("%s/1.fs" % tdata) zfs1 = _readfile("%s/1%s.fs" % (tdata, zkind))
zfs2 = _readfile("%s/2.fs" % tmpd) zfs2 = _readfile("%s/2.fs" % tmpd)
assert zfs1 == zfs2 assert zfs1 == zfs2
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment