Conversation
| assert resp.status_code == 200 | ||
| shards = resp.body["shards"] | ||
| assert node1 not in shards["00000000-ffffffff"] | ||
| # assert node1 not in shards["00000000-ffffffff"] |
There was a problem hiding this comment.
@nickva I've had to comment out this line. Does this still make the test valid?
There was a problem hiding this comment.
Let's replace it with
assert node1 not in Map.get(shards, "00000000-ffffffff", [])
This way it will still check that the original range was gone and work on n=1 and n>2 cases
(Added a bit of extra explanation in a similar assert below)
| shards = resp.body["shards"] | ||
| assert node1 not in shards["00000000-7fffffff"] | ||
| assert node1 not in shards["80000000-ffffffff"] | ||
| # assert node1 not in shards["00000000-7fffffff"] |
There was a problem hiding this comment.
@nickva I've had to comment out these two lines. Is the test still valid?
There was a problem hiding this comment.
Still valid. But maybe better to replace the checks with
assert node1 not in Map.get(shards, "00000000-7fffffff", [])
assert node1 not in Map.get(shards, "80000000-ffffffff", [])
What that means is before we were checking that after those ranges were split, the copies from the original node and range were gone. With other nodes in that range on node2 and node3, shards[00..-7f..] was returning a list of [node2, node3]. Then with n=1,there are no copies in the those ranges on any nodes so shards[...] was failing. The fix is to do a get with a default value of [], so it would work on both n=1 and n>1
There was a problem hiding this comment.
Awesome thanks @nickva. I've changed the tests.
|
Tests passed first time on Travis 🎉 |
|
@garrensmith temporary, or permanent change? |
|
This would be permanent
…________________________________
From: Joan Touzet <notifications@github.com>
Sent: Sunday, December 15, 2019 9:25 PM
To: apache/couchdb
Cc: garren smith; Mention
Subject: Re: [apache/couchdb] Switch make elixir to use n = 1 (#2363)
@garrensmith<https://github.com/garrensmith> temporary, or permanent change?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#2363?email_source=notifications&email_token=AABL2AXOGJJOEZDNSKFD4C3QY2ADHA5CNFSM4J27CAYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEG5AHBY#issuecomment-565838727>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AABL2AQS4W3WAB5Q5ZZAECLQY2ADHANCNFSM4J27CAYA>.
|
|
+1 until we rewrite the tests to be specifically n>1 compatible, we should keep it as one. going to n=3 without rewriting the tests was aspirational at best |
|
If the next release after 3.0 is with FDB, I don't think there would be much to gain by rewriting the tests to be n = 3. |
|
there will absolutely be a 3.x chain for a while in parallel with 4.0; whether 3.1 predates or postdates 4.0 is anyone's guess at this point. |
jaydoane
left a comment
jaydoane
left a comment
There was a problem hiding this comment.
LGTM:
$ make elixir
...
Finished in 37.5 seconds
293 tests, 0 failures, 26 excluded
davisp
force-pushed
the
change-elixir-tests-n-1
branch
from
f39bced to
9d2d7be
Compare
8 participants
Overview
Switch
make elixirto use n = 1. This matches how the javascript tests are run and hopefully should make the elixir tests a little more consistent and less flaky.Testing recommendations
Tests should pass on Travis and Jenkins
Related Issues or Pull Requests
Checklist
rel/overlay/etc/default.ini