Tarantool development patches archive
 help / color / mirror / Atom feed
From: Alexander Turenko <alexander.turenko@tarantool.org>
To: Igor Munkin <imun@tarantool.org>
Cc: tarantool-patches@freelists.org, Kirill Yukhin <kyukhin@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH 1/1] test: add tests from tarantool source tree
Date: Tue, 10 Sep 2019 18:55:10 +0300	[thread overview]
Message-ID: <20190910155510.ipvoaaffhhddceb7@tkn_work_nb> (raw)
In-Reply-To: <1568127797.352311221@f463.i.mail.ru>

LGTM, CCed Kirill.

WBR, Alexander Turenko.

On Tue, Sep 10, 2019 at 06:03:17PM +0300, Igor Munkin wrote:
> 
> Sasha,
> 
> Thanks for you review. 
> >Вторник, 10 сентября 2019, 16:58 +03:00 от Alexander Turenko <alexander.turenko@tarantool.org>:
> >
> >I found the following difference with the current master:
> >
> > | --- a/luajit-tap/fold_bug_LuaJIT_505.test.lua	2019-09-10 15:45:04.886639461 +0300
> > | +++ b/test/fold_bug_LuaJIT_505.test.lua	2019-09-10 15:44:22.221641210 +0300
> > | @@ -8,7 +8,8 @@
> > |  -- Test file to demonstrate Lua fold machinery icorrect behavior, details:
> > |  --  https://github.com/LuaJIT/LuaJIT/issues/505
> > | 
> > | -jit.opt.start("hotloop=1")
> > | +jit.opt.start(0, "hotloop=1")
> > | +require('jit.dump').start("+bti", "505.trace")
> > |  for _ = 1, 20 do
> > |      local value = "abc"
> > |      local pos_c = string.find(value, "c", 1, true)
> >
> >From what it appears? If it is necessary, then move it to its own commit
> >or describe in the commit message. 
>
> The difference you found is an artefact left after investigation of the problem related to string.find recording. These changes are not required within this patch.

Okay.

> >
> >
> >Please, base your branch on top of a last commit in a default branch
> >(named 'tarantool'). Now it contains one extra commit ('luajit: fix
> >string.find recording').
> Another artefact.

Okay.

> >
> >
> >Verified in-source build, works ok.
> >
> >Tried out-of-source build: small and luajit-tap tests are not run (just
> >not found).
> >
> >If we'll create just symlinks in 'test' repository directory and will
> >remove creating symlinks from test/CMakeLists.txt, then luajit-tap would
> >work good, but small tests would not be built (and so small tests will
> >be found).
> >
> >It seems it is subject for a separate issue: don't sure about test-run
> >or tarantool. Something was changed in test-run I guess; maybe here:
> >https://github.com/tarantool/test-run/commit/b42093abc0e7fe895ff0ac7c0685d016c783b3a1
> >
> >Don't sure whether it worth to block on this problem or file an issue
> >and going on. Please, ask Kirill for that.

It is about the patch to tarantool more; let's skip it here.

Anyway, I filed https://github.com/tarantool/tarantool/issues/4485 to
investigate it deeper a bit later.

> >
> >Other then that everything look okay for me. 
> I've already fixed all mentioned flaws in the feature branch and forcely update the corresponding remote branch. Please inform me whether a new patch is required to be sent.

Okay.

> >On Thu, Sep 05, 2019 at 05:20:07PM +0300, Igor Munkin wrote:
> >> Introduced structure has the following benefits:
> >> * Excess testing machinery currently is not required directly in
> >> luajit repo considering whole CI process setup for tarantool and its
> >> static linking with libluajit
> >> * All new tests related to luajit fixes and enhancements can be added
> >> within a single patch with the corresponding changeset
> >> 
> >> NB!: All added test chunks use tarantool tap module
> >> ---
> >>  test/fold_bug_LuaJIT_505.test.lua        |  21 +++
> >>  test/gh.test.lua                         |  17 +++
> >>  test/suite.ini                           |   5 +
> >>  test/table_chain_bug_LuaJIT_494.test.lua | 178 +++++++++++++++++++++++
> >>  test/unsink_64_kptr.test.lua             |  44 ++++++
> >>  5 files changed, 265 insertions(+)
> >>  create mode 100755 test/fold_bug_LuaJIT_505.test.lua
> >>  create mode 100755 test/gh.test.lua
> >>  create mode 100644 test/suite.ini
> >>  create mode 100755 test/table_chain_bug_LuaJIT_494.test.lua
> >>  create mode 100755 test/unsink_64_kptr.test.lua
> 
> -- 
> IM

      reply	other threads:[~2019-09-10 15:55 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-05 14:20 [tarantool-patches] [PATCH 0/1] Move luajit-related tests to luajit repo Igor Munkin
2019-09-05 14:20 ` [tarantool-patches] [PATCH 1/1] test: add tests from tarantool source tree Igor Munkin
2019-09-10 13:57   ` [tarantool-patches] " Alexander Turenko
2019-09-10 15:03     ` [tarantool-patches] Re[2]: [tarantool-patches] " Igor Munkin
2019-09-10 15:55       ` Alexander Turenko [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190910155510.ipvoaaffhhddceb7@tkn_work_nb \
    --to=alexander.turenko@tarantool.org \
    --cc=imun@tarantool.org \
    --cc=kyukhin@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH 1/1] test: add tests from tarantool source tree' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox