Strengthen FilterRoutingTests #33149
Conversation
Today the FilterRoutingTests take the belt-and-braces approach of excluding some node attribute values and including some others. This means that we don't really test that both inclusion and exclusion work correctly: as long as one of them works as expected then the test will pass. This change improves these tests by only using one approach at once, demonstrating that both do indeed work.
DaveCTurner
added
>non-issue
v7.0.0
:Distributed Coordination/Allocation
|
Pinging @elastic/es-distributed |
| .build())) | ||
| .build(); | ||
| final Settings.Builder settingsBuilder = settings(Version.CURRENT).put("index.number_of_shards", 2).put("index.number_of_replicas", 1); | ||
| if (randomBoolean()) { |
There was a problem hiding this comment.
I like what you're doing here to improve these tests. However, I have been moving away from using randomness as a poor man's way to get branch coverage. I think that randomness is good for testing edge cases by randomly varying input data, but I don't like seeing it to half of the time cover one branch and half of the time cover another. I would prefer to see two tests here.
|
Seems reasonable. This kinda snowballed as I dug into the implementation and realised there were quite a lot of things that aren't very well covered at the moment:
Some of these are somewhat covered at a lower level by |
|
Build failed due to issue described in #29140. @elasticmachine retest this please. |
Today the FilterRoutingTests take the belt-and-braces approach of excluding some node attribute values and including some others. This means that we don't really test that both inclusion and exclusion work correctly: as long as one of them works as expected then the test will pass. This change improves these tests by only using one approach at once, demonstrating that both do indeed work, and adds tests for various other scenarios too.
* master: Add full cluster restart base class (elastic#33577) Validate list values for settings (elastic#33503) Copy and validatie soft-deletes setting on resize (elastic#33517) Test: Fix package name SQL: Fix result column names for arithmetic functions (elastic#33500) Upgrade to latest Lucene snapshot (elastic#33505) Enable not wiping cluster settings after REST test (elastic#33575) MINOR: Remove Dead Code in SearchScript (elastic#33569) [Test] Remove duplicate method in TestShardRouting (elastic#32815) mute test on windows Update beats template to include apm-server metrics (elastic#33286) Fix typos (elastic#33499) [CCR] Delay auto follow license check (elastic#33557) [CCR] Add create_follow_index privilege (elastic#33559) Strengthen FilterRoutingTests (elastic#33149) Correctly handle PKCS#11 tokens for system keystore (elastic#33460) Remove some duplicate request conversion methods. (elastic#33538)
Today the FilterRoutingTests take the belt-and-braces approach of excluding
some node attribute values and including some others. This means that we don't
really test that both inclusion and exclusion work correctly: as long as one of
them works as expected then the test will pass. This change improves these
tests by only using one approach at once, demonstrating that both do indeed
work.