<div dir="auto"><div>OK, no objections then.<br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, 21 May 2021, 22:26 Vladislav Shpilevoy, <<a href="mailto:v.shpilevoy@tarantool.org">v.shpilevoy@tarantool.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi! Thanks for the review!<br>
<br>
On 19.05.2021 23:50, Yaroslav Dynnikov wrote:<br>
> HI, Vlad<br>
> <br>
> Thanks for the patch.<br>
> Find one comment below.<br>
> <br>
> On Thu, 13 May 2021 at 14:07, Vladislav Shpilevoy <<a href="mailto:v.shpilevoy@tarantool.org" target="_blank" rel="noreferrer">v.shpilevoy@tarantool.org</a> <mailto:<a href="mailto:v.shpilevoy@tarantool.org" target="_blank" rel="noreferrer">v.shpilevoy@tarantool.org</a>>> wrote:<br>
> <br>
>     Duplicate key error at insertion into a space on the latest<br>
>     Tarantool changed its message and it broke of the tests. The patch<br>
>     updates the test so it checks only the needed part of the message<br>
>     and does not depend on Tarantool version anymore.<br>
>     ---<br>
>      test/storage/storage.result   | 8 +++++---<br>
>      test/storage/storage.test.lua | 3 ++-<br>
>      2 files changed, 7 insertions(+), 4 deletions(-)<br>
> <br>
>     diff --git a/test/storage/storage.result b/test/storage/storage.result<br>
>     index 2c9784a..d18b7f8 100644<br>
>     --- a/test/storage/storage.result<br>
>     +++ b/test/storage/storage.result<br>
>     @@ -179,10 +179,12 @@ vshard.storage.buckets_info()<br>
>          status: active<br>
>          id: 1<br>
>      ...<br>
>     -vshard.storage.bucket_force_create(1) -- error<br>
>     +ok, err = vshard.storage.bucket_force_create(1)<br>
>      ---<br>
>     -- null<br>
>     -- Duplicate key exists in unique index 'pk' in space '_bucket'<br>
>     +...<br>
>     +assert(not ok and err.message:match("Duplicate key exists"))<br>
>     +---<br>
>     +- Duplicate key exists<br>
>      ...<br>
>      vshard.storage.bucket_force_drop(1)<br>
>      ---<br>
>     diff --git a/test/storage/storage.test.lua b/test/storage/storage.test.lua<br>
>     index 33f0498..97558f6 100644<br>
>     --- a/test/storage/storage.test.lua<br>
>     +++ b/test/storage/storage.test.lua<br>
>     @@ -56,7 +56,8 @@ vshard.storage.sync(100500)<br>
>      vshard.storage.buckets_info()<br>
>      vshard.storage.bucket_force_create(1)<br>
>      vshard.storage.buckets_info()<br>
>     -vshard.storage.bucket_force_create(1) -- error<br>
>     +ok, err = vshard.storage.bucket_force_create(1)<br>
>     +assert(not ok and "err.message:match("Duplicate key exists))<br>
> <br>
> <br>
> I'd suggest splitting the check in two:<br>
> <br>
> 1. ok -- should be false<br>
> 2. Then check the message matches.<br>
> <br>
> Assertions usually don't provide useful errors.<br>
<br>
In the core we try to use assertions more, because this makes the<br>
tests easier to read. You can see right away what are the target<br>
points of the test. This is kind of a simulation of tap tests.<br>
<br>
I consider vshard be "half-core", so I decided to use assertions<br>
here as well.<br>
</blockquote></div></div></div>