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