DonalEvans commented on a change in pull request #7164:
URL: https://github.com/apache/geode/pull/7164#discussion_r762167146
##########
File path:
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSIsMemberIntegrationTest.java
##########
@@ -49,42 +50,81 @@ public void tearDown() {
}
@Test
- public void errors_givenWrongNumberOfArguments() {
+ public void sismemberWrongNumberOfArguments_returnsError() {
assertExactNumberOfArgs(jedis, Protocol.Command.SISMEMBER, 2);
}
@Test
- public void testSMembersSIsMember() {
- int elements = 10;
- String key = generator.generate('x');
- String[] strings = generateStrings(elements, 'y');
- jedis.sadd(key, strings);
-
- for (String entry : strings) {
- assertThat(jedis.sismember(key, entry)).isTrue();
+ public void sismemberValidKeyValidMember_returnTrue() {
+ jedis.sadd(setKey, setMembers);
+
+ for (String member : setMembers) {
+ assertThat(jedis.sismember(setKey, member)).isTrue();
}
}
@Test
- public void testSIsMemberWithNonexistentKey_returnsFalse() {
+ public void sismemberValidKeyNonExistingMember_returnFalse() {
+ jedis.sadd(setKey, setMembers);
+ assertThat(jedis.sismember(setKey, "nonExistentMember")).isFalse();
+ }
+
+ @Test
+ public void sismemberNonExistingKeyNonExistingMember_returnFalse() {
assertThat(jedis.sismember("nonExistentKey",
"nonExistentMember")).isFalse();
}
@Test
- public void testSIsMemberWithNonexistentMember_returnsFalse() {
- String key = "key";
+ public void sismemberMemberInAnotherSet_returnFalse() {
+ String member = "elephant";
+ jedis.sadd("diffSet", member);
+ jedis.sadd(setKey, setMembers);
- jedis.sadd("key", "member1", "member2");
- assertThat(jedis.sismember(key, "nonExistentMember")).isFalse();
+ assertThat(jedis.sismember(setKey, member)).isFalse();
}
- private String[] generateStrings(int elements, char uniqueElement) {
- Set<String> strings = new HashSet<>();
- for (int i = 0; i < elements; i++) {
- String elem = generator.generate(uniqueElement);
- strings.add(elem);
+ @Test
+ public void sismemberNonExistingKeyAndMemberInAnotherSet_returnFalse() {
+ jedis.sadd(setKey, setMembers);
+
+ for (String member : setMembers) {
+ assertThat(jedis.sismember("nonExistentKey", member)).isFalse();
}
- return strings.toArray(new String[strings.size()]);
}
+
+ @Test
+ public void sismemberAfterSadd_returnsTruee() {
Review comment:
Typo here, should be "True"
##########
File path:
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/set/AbstractSIsMemberIntegrationTest.java
##########
@@ -49,42 +50,81 @@ public void tearDown() {
}
@Test
- public void errors_givenWrongNumberOfArguments() {
+ public void sismemberWrongNumberOfArguments_returnsError() {
assertExactNumberOfArgs(jedis, Protocol.Command.SISMEMBER, 2);
}
@Test
- public void testSMembersSIsMember() {
- int elements = 10;
- String key = generator.generate('x');
- String[] strings = generateStrings(elements, 'y');
- jedis.sadd(key, strings);
-
- for (String entry : strings) {
- assertThat(jedis.sismember(key, entry)).isTrue();
+ public void sismemberValidKeyValidMember_returnTrue() {
+ jedis.sadd(setKey, setMembers);
+
+ for (String member : setMembers) {
+ assertThat(jedis.sismember(setKey, member)).isTrue();
}
}
@Test
- public void testSIsMemberWithNonexistentKey_returnsFalse() {
+ public void sismemberValidKeyNonExistingMember_returnFalse() {
+ jedis.sadd(setKey, setMembers);
+ assertThat(jedis.sismember(setKey, "nonExistentMember")).isFalse();
+ }
+
+ @Test
+ public void sismemberNonExistingKeyNonExistingMember_returnFalse() {
assertThat(jedis.sismember("nonExistentKey",
"nonExistentMember")).isFalse();
}
@Test
- public void testSIsMemberWithNonexistentMember_returnsFalse() {
- String key = "key";
+ public void sismemberMemberInAnotherSet_returnFalse() {
+ String member = "elephant";
+ jedis.sadd("diffSet", member);
+ jedis.sadd(setKey, setMembers);
- jedis.sadd("key", "member1", "member2");
- assertThat(jedis.sismember(key, "nonExistentMember")).isFalse();
+ assertThat(jedis.sismember(setKey, member)).isFalse();
}
- private String[] generateStrings(int elements, char uniqueElement) {
- Set<String> strings = new HashSet<>();
- for (int i = 0; i < elements; i++) {
- String elem = generator.generate(uniqueElement);
- strings.add(elem);
+ @Test
+ public void sismemberNonExistingKeyAndMemberInAnotherSet_returnFalse() {
+ jedis.sadd(setKey, setMembers);
+
+ for (String member : setMembers) {
+ assertThat(jedis.sismember("nonExistentKey", member)).isFalse();
}
- return strings.toArray(new String[strings.size()]);
}
+
+ @Test
+ public void sismemberAfterSadd_returnsTruee() {
+ String newMember = "chicken";
+ jedis.sadd(setKey, setMembers);
+ assertThat(jedis.sismember(setKey, newMember)).isFalse();
+ jedis.sadd(setKey, newMember);
+ assertThat(jedis.sismember(setKey, newMember)).isTrue();
+ }
+
+ @Test
+ public void sismemberWithConcurrentSAdd_returnsCorrectValue() {
Review comment:
For this specific command a concurrent test doesn't really make sense,
since we're asserting that SISMEMBER either returns true or it returns false
depending on whether it's called before or after the SADD. However, since
there's nothing else that the command could return, we have no way of detecting
a concurrency issue, since one of those assertions will always be correct. For
this reason, I think it makes sense to just remove this test case.
--
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]
