[Tarantool-patches] [PATCH v2 08/10] test: support tarantool cli in lua-Harness
Sergey Kaplun
skaplun at tarantool.org
Wed Jul 28 21:44:14 MSK 2021
Hi, Maxim!
Thanks for the patch and fixes!
LGTM, except a few nits regarding to the commit message.
On 26.07.21, Максим Корякшин wrote:
>
> Thanks for the review, Igor!
> Here is the new commit message, considering your comments:
> =================================================
> test: support tarantool cli in lua-Harness
>
> The patch[1] from lua-Harness suite adjusts some checks in
Typo: s/from lua-Harness suite/the from lua-Harness suite/
> lua-Harness tests, so they are compatible with Tarantool now.
>
> As a result, the introduced assertion for Tarantool error message fails
> on MacOS, since getopt_long(3) yields the error message without single
> quotes wrapping the flag. Hence, this commit also includes fix for that
Typo: s/fix/the fix/
> from patch[2] in the trunk.
Typo: s/patch/the patch/
>
> [1]: https://framagit.org/fperrad/lua-Harness/-/commit/1da5b1b
> [2]: https://framagit.org/fperrad/lua-Harness/-/commit/a0532c5
>
> Part of tarantool/tarantool#5970
> Part of tarantool/tarantool#4473
>
> =================================================
>
>
> >Понедельник, 26 июля 2021, 0:37 +03:00 от Igor Munkin <imun at tarantool.org>:
> >
> >Max,
> >
> >Thanks for the patch! The changes are fine, but please consider the
> >comments below regarding the commit message.
> >
> >On 20.07.21, Maxim Kokryashkin wrote:
> >> The patch[1] from lua-Harness suite adds some specific checks to
> >> lua-Harness tests, so they are compatible with Tarantool now.
> >
> >Strictly saying, no *new* checks are *added*, but the *existing* ones
> >are *adjusted* considering Tarantool specifics.
> >
> >>
> >> Considering this, commit d4e12d7ac28e3bc857d30971dd77deec66a67297('test:
> >> disable LuaJIT CLI tests in lua-Harness suite'), which disabled
> >> tarantool cli tests, can be superseded.
> >
> >No, it can't. The mentioned commit is superseded by "[PATCH v2 06/10]
> >test: support tarantool in lua-Harness" in this series.
> >
> >>
> >> As a result, the introduced assertion for Tarantool error message fails
> >> on MacOS, since getopt_long(3) yields the error message without single
> >> quotes wrapping the flag. Hence, this commit also includes fix for that
> >> from patch[2] in the trunk.
> >>
> >> [1]: https://framagit.org/fperrad/lua-Harness/-/commit/1da5b1b3
> >
> >Minor: 7 digits are enough. This is not critical, but just odd to have
> >two similar links with the different number of digits, so feel free to
> >ignore this nit.
> >
> >> [2]: https://framagit.org/fperrad/lua-Harness/-/commit/a0532c5
> >
> >Please mention the related issues: #5970 and #4473.
> >
> >> ---
> >> Additional information on issue with MacOS:
> >> https://github.com/tarantool/tarantool/issues/5970#issuecomment-880158605
> >>
> >> NOTICE: tests for this separate commit are failing on FreeBSD
> >>
> >> test/lua-Harness-tests/241-standalone.t | 79 +++++++++++++++++++------
> >> 1 file changed, 61 insertions(+), 18 deletions(-)
> >>
> >
> ><snipped>
> >
> >> --
> >> 2.32.0
> >>
> >
> >--
> >Best regards,
> >IM
>
--
Best regards,
Sergey Kaplun
More information about the Tarantool-patches
mailing list