SOLR-11170: Make AssignBackwardCompatibilityTest more reliable

solr/core/src/java/org/apache/solr/util/NumberUtils.java

@@ -211,11 +211,7 @@ public class NumberUtils {
 
   public static int bytesToInt(byte[] bytes) {
     if (bytes == null) return 0;
-    int val = 0;
+    assert bytes.length == 4;
-    for (int i = 0; i < bytes.length; i++) {
+    return bytes[0] << 24 | (bytes[1] & 255) << 16 | (bytes[2] & 255) << 8 | bytes[3] & 255;
-      val = val << 8;
-      val += bytes[i];
-    }
-    return val;
   }
 }

solr/core/src/test/org/apache/solr/cloud/AssignBackwardCompatibilityTest.java

@@ -28,7 +28,9 @@ import org.apache.solr.client.solrj.response.CollectionAdminResponse;
 import org.apache.solr.common.cloud.ClusterStateUtil;
 import org.apache.solr.common.cloud.DocCollection;
 import org.apache.solr.common.cloud.Replica;
+import org.apache.solr.util.NumberUtils;
 import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.data.Stat;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.slf4j.Logger;
@@ -59,21 +61,22 @@ public class AssignBackwardCompatibilityTest extends SolrCloudTestCase {
     Set<String> coreNames = new HashSet<>();
     Set<String> coreNodeNames = new HashSet<>();
 
-    int numOperations = random().nextInt(4 * 15);
+    int numOperations = random().nextInt(15) + 15;
     int numLiveReplicas = 4;
 
     boolean clearedCounter = false;
     for (int i = 0; i < numOperations; i++) {
+      log.info("Collection counter={} i={}", getCounter(), i);
       boolean deleteReplica = random().nextBoolean() && numLiveReplicas > 1;
       // No need to clear counter more than one time
-      if (random().nextInt(30) < 5 && !clearedCounter) {
+      if (random().nextBoolean() && i > 5 && !clearedCounter) {
+        log.info("Clear collection counter");
         // clear counter
         cluster.getZkClient().delete("/collections/"+COLLECTION+"/counter", -1, true);
         clearedCounter = true;
       }
       if (deleteReplica) {
-        assertTrue(ClusterStateUtil.waitForLiveAndActiveReplicaCount(
+        waitForState("Expected " + numLiveReplicas + " active replicas", COLLECTION, clusterShape(1, numLiveReplicas));
-            cluster.getSolrClient().getZkStateReader(), COLLECTION, numLiveReplicas, 30000));
         DocCollection dc = getCollectionState(COLLECTION);
         Replica replica = getRandomReplica(dc.getSlice("shard1"), (r) -> r.getState() == Replica.State.ACTIVE);
         CollectionAdminRequest.deleteReplica(COLLECTION, "shard1", replica.getName()).process(cluster.getSolrClient());
@@ -86,6 +89,8 @@ public class AssignBackwardCompatibilityTest extends SolrCloudTestCase {
             .keySet().iterator().next();
         assertFalse("Core name is not unique coreName=" + coreName + " " + coreNames, coreNames.contains(coreName));
         coreNames.add(coreName);
+        numLiveReplicas++;
+        waitForState("Expected " + numLiveReplicas + " active replicas", COLLECTION, clusterShape(1, numLiveReplicas));
 
         Replica newReplica = getCollectionState(COLLECTION).getReplicas().stream()
             .filter(r -> r.getCoreName().equals(coreName))
@@ -93,9 +98,18 @@ public class AssignBackwardCompatibilityTest extends SolrCloudTestCase {
         String coreNodeName = newReplica.getName();
         assertFalse("Core node name is not unique", coreNodeNames.contains(coreName));
         coreNodeNames.add(coreNodeName);
-
-        numLiveReplicas++;
       }
     }
   }
+
+  private int getCounter() throws KeeperException, InterruptedException {
+    try {
+      byte[] data = cluster.getZkClient().getData("/collections/"+COLLECTION+"/counter", null, new Stat(), true);
+      int count = NumberUtils.bytesToInt(data);
+      if (count < 0) throw new AssertionError("Found negative collection counter " + count);
+      return count;
+    } catch (KeeperException e) {
+      return -1;
+    }
+  }
 }

solr/core/src/test/org/apache/solr/util/NumberUtilsTest.java

@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.util;
+
+import org.apache.solr.SolrTestCaseJ4;
+import org.junit.Test;
+
+public class NumberUtilsTest extends SolrTestCaseJ4 {
+  @Test
+  public void testIntToBytes() {
+    assertEquals(0, NumberUtils.bytesToInt(null));
+    assertEquals(-1, encodeAndDecode(-1));
+    assertEquals(-100, encodeAndDecode(-100));
+    assertEquals(-1000, encodeAndDecode(-1000));
+    assertEquals(0, encodeAndDecode(0));
+    assertEquals(100, encodeAndDecode(100));
+    assertEquals(1000, encodeAndDecode(1000));
+    assertEquals(Integer.MAX_VALUE, encodeAndDecode(Integer.MAX_VALUE));
+    assertEquals(Integer.MIN_VALUE, encodeAndDecode(Integer.MIN_VALUE));
+  }
+
+  public int encodeAndDecode(int i) {
+    return NumberUtils.bytesToInt(NumberUtils.intToBytes(i));
+  }
+}