Strengthen FilterRoutingTests (#33149)

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.
elastic · Sep 10, 2018 · fc5da93 · fc5da93
1 parent 730fb46
commit fc5da93
Show file tree

Hide file tree

Showing 3 changed files with 245 additions and 68 deletions.
diff --git a/buildSrc/src/main/resources/checkstyle_suppressions.xml b/buildSrc/src/main/resources/checkstyle_suppressions.xml
@@ -499,7 +499,6 @@
   <suppress files="server[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]cluster[/\\]routing[/\\]allocation[/\\]ExpectedShardSizeAllocationTests.java" checks="LineLength" />
   <suppress files="server[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]cluster[/\\]routing[/\\]allocation[/\\]FailedNodeRoutingTests.java" checks="LineLength" />
   <suppress files="server[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]cluster[/\\]routing[/\\]allocation[/\\]FailedShardsRoutingTests.java" checks="LineLength" />
-  <suppress files="server[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]cluster[/\\]routing[/\\]allocation[/\\]FilterRoutingTests.java" checks="LineLength" />
   <suppress files="server[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]cluster[/\\]routing[/\\]allocation[/\\]IndexBalanceTests.java" checks="LineLength" />
   <suppress files="server[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]cluster[/\\]routing[/\\]allocation[/\\]NodeVersionAllocationDeciderTests.java" checks="LineLength" />
   <suppress files="server[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]cluster[/\\]routing[/\\]allocation[/\\]PreferLocalPrimariesToRelocatingPrimariesTests.java" checks="LineLength" />

diff --git a/...in/java/org/elasticsearch/cluster/routing/allocation/decider/FilterAllocationDecider.java b/...in/java/org/elasticsearch/cluster/routing/allocation/decider/FilterAllocationDecider.java
@@ -158,13 +158,13 @@ private Decision shouldFilter(IndexMetaData indexMd, RoutingNode node, RoutingAl
 
     private Decision shouldIndexFilter(IndexMetaData indexMd, RoutingNode node, RoutingAllocation allocation) {
         if (indexMd.requireFilters() != null) {
-            if (!indexMd.requireFilters().match(node.node())) {
+            if (indexMd.requireFilters().match(node.node()) == false) {
                 return allocation.decision(Decision.NO, NAME, "node does not match index setting [%s] filters [%s]",
                     IndexMetaData.INDEX_ROUTING_REQUIRE_GROUP_PREFIX, indexMd.requireFilters());
             }
         }
         if (indexMd.includeFilters() != null) {
-            if (!indexMd.includeFilters().match(node.node())) {
+            if (indexMd.includeFilters().match(node.node()) == false) {
                 return allocation.decision(Decision.NO, NAME, "node does not match index setting [%s] filters [%s]",
                     IndexMetaData.INDEX_ROUTING_INCLUDE_GROUP_PREFIX, indexMd.includeFilters());
             }
@@ -180,13 +180,13 @@ private Decision shouldIndexFilter(IndexMetaData indexMd, RoutingNode node, Rout
 
     private Decision shouldClusterFilter(RoutingNode node, RoutingAllocation allocation) {
         if (clusterRequireFilters != null) {
-            if (!clusterRequireFilters.match(node.node())) {
+            if (clusterRequireFilters.match(node.node()) == false) {
                 return allocation.decision(Decision.NO, NAME, "node does not match cluster setting [%s] filters [%s]",
                     CLUSTER_ROUTING_REQUIRE_GROUP_PREFIX, clusterRequireFilters);
             }
         }
         if (clusterIncludeFilters != null) {
-            if (!clusterIncludeFilters.match(node.node())) {
+            if (clusterIncludeFilters.match(node.node()) == false) {
                 return allocation.decision(Decision.NO, NAME, "node does not cluster setting [%s] filters [%s]",
                     CLUSTER_ROUTING_INCLUDE_GROUP_PREFIX, clusterIncludeFilters);
             }