jdeppe-pivotal commented on a change in pull request #7340:
URL: https://github.com/apache/geode/pull/7340#discussion_r806047523
##########
File path:
geode-for-redis/src/commonTest/java/org/apache/geode/redis/mocks/MockSubscriber.java
##########
@@ -45,8 +43,9 @@
Collections.synchronizedList(new ArrayList<>());
private CountDownLatch messageReceivedLatch = new CountDownLatch(0);
private CountDownLatch pMessageReceivedLatch = new CountDownLatch(0);
- private String localSocketAddress;
- private Client client;
+ /*
+ * private String localSocketAddress;
+ */
Review comment:
Probably delete this.
##########
File path:
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/sortedset/AbstractZUnionStoreIntegrationTest.java
##########
@@ -234,20 +235,20 @@ public void
shouldUnionize_givenBoundaryScoresAndWeights() {
jedis.zunionstore(NEW_SET, new ZParams().weights(1), KEY1);
- Set<Tuple> results = jedis.zrangeWithScores(NEW_SET, 0, scores.size());
- assertThat(results).containsExactlyElementsOf(expectedResults);
+ List<Tuple> results = jedis.zrangeWithScores(NEW_SET, 0, scores.size());
+ assertThat(results).containsExactlyInAnyOrderElementsOf(expectedResults);
Review comment:
Please keep all of these assertions as `containsExactlyElementsOf` if
possible (I ran a quick test and looks like it's OK). Since `ZRANGESCORE` is
supposed to return an ordered list, we should check that.
##########
File path:
geode-for-redis/src/commonTest/java/org/apache/geode/redis/mocks/MockSubscriber.java
##########
@@ -65,12 +64,24 @@ public MockSubscriber(CountDownLatch subscriptionLatch,
CountDownLatch unsubscri
this.pUnsubscriptionLatch = pUnsubscriptionLatch;
}
- @Override
- public void proceed(Client client, String... channels) {
- localSocketAddress = client.getSocket().getLocalSocketAddress().toString();
- this.client = client;
- super.proceed(client, channels);
- }
+ /*
+ * @Override
+ * public void proceed(final Connection connection, final String...
channels) {
+ * try {
+ * // Kludge due to socket becoming private in jedis 4.1.1
+ * // TODO is there a safe public way of getting local socket address
+ * // This doesn't work (or no longer works in Jedis 4.1.1), results in null
socket
+ * final Field privateSocketField =
Connection.class.getDeclaredField("socket");
+ * privateSocketField.setAccessible(true);
+ * final Socket socket = (Socket) privateSocketField.get(connection);
+ *
+ * localSocketAddress = socket.getLocalSocketAddress().toString();
+ * } catch (final NoSuchFieldException | IllegalAccessException ex) {
+ * throw new RuntimeException("Error in accessing private field 'socket' via
reflection", ex);
+ * }
+ * super.proceed(connection, channels);
+ * }
+ */
Review comment:
Also deleted.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
