kirklund commented on a change in pull request #6393:
URL: https://github.com/apache/geode/pull/6393#discussion_r650148533
##########
File path:
geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java
##########
@@ -162,17 +159,17 @@ public void testAttributeDesc() {
@Test
public void sameCount() {
- assertEquals(attributes.size(), setters.size());
- assertEquals(setters.size(), getters.size());
+ assertThat(setters).hasSize(attributes.size());
+ assertThat(getters).hasSize(setters.size());
Review comment:
There is a `hasSameSizeAs` assertion that is cleaner and provides
better failure messages:
```
assertThat(setters).hasSameSizeAs(attributes);
assertThat(getters).hasSameSizeAs(setters);
```
##########
File path:
geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java
##########
@@ -235,55 +232,59 @@ public void everyGetterSetterSameNameSameType() {
for (String attr : getters.keySet()) {
Method getter = getters.get(attr);
Method setter = setters.get(attr);
- assertNotNull("every getter should have a corresponding setter " + attr,
setter);
+ assertThat(setter).as("every getter should have a corresponding setter "
+ attr).isNotNull();
String setterName = setter.getName();
String getterName = getter.getName();
- assertEquals(setterName.substring(setterName.indexOf("set") + 3),
- getterName.substring(getterName.indexOf("get") + 3));
- assertEquals(setter.getParameterTypes()[0], getter.getReturnType());
+ assertThat(getterName.substring(getterName.indexOf("get") + 3))
+ .isEqualTo(setterName.substring(setterName.indexOf("set") + 3));
+
assertThat(getter.getReturnType()).isEqualTo(setter.getParameterTypes()[0]);
}
for (String attr : setters.keySet()) {
Method getter = getters.get(attr);
- assertNotNull("every setter should have a corresponding getter: " +
attr, getter);
+ assertThat(getter).as("every setter should have a corresponding getter:
" + attr).isNotNull();
}
}
@Test
public void everySetterHasAttributeDefined() {
for (String attr : setters.keySet()) {
ConfigAttribute configAttribute = attributes.get(attr);
- assertNotNull(attr + " should be defined a ConfigAttribute",
configAttribute);
+ assertThat(configAttribute).as(attr + " should be defined a
ConfigAttribute").isNotNull();
}
}
@Test
public void everyGetterHasAttributeDefined() {
for (String attr : getters.keySet()) {
ConfigAttribute configAttribute = attributes.get(attr);
- assertNotNull(attr + " should be defined a ConfigAttribute",
configAttribute);
+ assertThat(configAttribute).as(attr + " should be defined a
ConfigAttribute").isNotNull();
}
}
@Test
public void testGetAttributeObject() {
- assertEquals(config.getAttributeObject(LOG_LEVEL), "config");
- assertEquals(config.getAttributeObject(SECURITY_LOG_LEVEL), "config");
- assertEquals(config.getAttributeObject(REDUNDANCY_ZONE), "");
-
assertEquals(config.getAttributeObject(ENABLE_CLUSTER_CONFIGURATION).getClass(),
Boolean.class);
+ assertThat(config.getAttributeObject(LOG_LEVEL)).isEqualTo("config");
+
assertThat(config.getAttributeObject(SECURITY_LOG_LEVEL)).isEqualTo("config");
+ assertThat(config.getAttributeObject(REDUNDANCY_ZONE)).isEqualTo("");
+
assertThat(config.getAttributeObject(ENABLE_CLUSTER_CONFIGURATION).getClass())
+ .isEqualTo(Boolean.class);
Review comment:
Better assertion syntax is:
```
assertThat(config.getAttributeObject(ENABLE_CLUSTER_CONFIGURATION)).isInstanceOf(Boolean.class);
```
##########
File path:
geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java
##########
@@ -235,55 +232,59 @@ public void everyGetterSetterSameNameSameType() {
for (String attr : getters.keySet()) {
Method getter = getters.get(attr);
Method setter = setters.get(attr);
- assertNotNull("every getter should have a corresponding setter " + attr,
setter);
+ assertThat(setter).as("every getter should have a corresponding setter "
+ attr).isNotNull();
String setterName = setter.getName();
String getterName = getter.getName();
- assertEquals(setterName.substring(setterName.indexOf("set") + 3),
- getterName.substring(getterName.indexOf("get") + 3));
- assertEquals(setter.getParameterTypes()[0], getter.getReturnType());
+ assertThat(getterName.substring(getterName.indexOf("get") + 3))
+ .isEqualTo(setterName.substring(setterName.indexOf("set") + 3));
+
assertThat(getter.getReturnType()).isEqualTo(setter.getParameterTypes()[0]);
Review comment:
Another String assertion syntax that works better is:
```
assertThat("foobar").contains("foo");
```
I would lean away from using`substring` and `indexOf` within assertions.
##########
File path:
geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java
##########
@@ -162,17 +159,17 @@ public void testAttributeDesc() {
@Test
public void sameCount() {
- assertEquals(attributes.size(), setters.size());
- assertEquals(setters.size(), getters.size());
+ assertThat(setters).hasSize(attributes.size());
+ assertThat(getters).hasSize(setters.size());
}
@Test
public void everyAttrHasValidSetter() {
for (String attr : attributes.keySet()) {
Method setter = setters.get(attr);
- assertNotNull(attr + " should have a setter", setter);
- assertTrue(setter.getName().startsWith("set"));
- assertEquals(setter.getParameterCount(), 1);
+ assertThat(setter).as(attr + " should have a setter").isNotNull();
+ assertThat(setter.getName().startsWith("set")).isTrue();
Review comment:
A better way to form this assertion with better failure message:
```
assertThat(setter.getName()).startsWith("set");
```
##########
File path:
geode-core/src/test/java/org/apache/geode/distributed/internal/DistributionConfigJUnitTest.java
##########
@@ -235,55 +232,59 @@ public void everyGetterSetterSameNameSameType() {
for (String attr : getters.keySet()) {
Method getter = getters.get(attr);
Method setter = setters.get(attr);
- assertNotNull("every getter should have a corresponding setter " + attr,
setter);
+ assertThat(setter).as("every getter should have a corresponding setter "
+ attr).isNotNull();
String setterName = setter.getName();
String getterName = getter.getName();
- assertEquals(setterName.substring(setterName.indexOf("set") + 3),
- getterName.substring(getterName.indexOf("get") + 3));
- assertEquals(setter.getParameterTypes()[0], getter.getReturnType());
+ assertThat(getterName.substring(getterName.indexOf("get") + 3))
+ .isEqualTo(setterName.substring(setterName.indexOf("set") + 3));
+
assertThat(getter.getReturnType()).isEqualTo(setter.getParameterTypes()[0]);
}
for (String attr : setters.keySet()) {
Method getter = getters.get(attr);
- assertNotNull("every setter should have a corresponding getter: " +
attr, getter);
+ assertThat(getter).as("every setter should have a corresponding getter:
" + attr).isNotNull();
}
}
@Test
public void everySetterHasAttributeDefined() {
for (String attr : setters.keySet()) {
ConfigAttribute configAttribute = attributes.get(attr);
- assertNotNull(attr + " should be defined a ConfigAttribute",
configAttribute);
+ assertThat(configAttribute).as(attr + " should be defined a
ConfigAttribute").isNotNull();
}
}
@Test
public void everyGetterHasAttributeDefined() {
for (String attr : getters.keySet()) {
ConfigAttribute configAttribute = attributes.get(attr);
- assertNotNull(attr + " should be defined a ConfigAttribute",
configAttribute);
+ assertThat(configAttribute).as(attr + " should be defined a
ConfigAttribute").isNotNull();
}
}
@Test
public void testGetAttributeObject() {
- assertEquals(config.getAttributeObject(LOG_LEVEL), "config");
- assertEquals(config.getAttributeObject(SECURITY_LOG_LEVEL), "config");
- assertEquals(config.getAttributeObject(REDUNDANCY_ZONE), "");
-
assertEquals(config.getAttributeObject(ENABLE_CLUSTER_CONFIGURATION).getClass(),
Boolean.class);
+ assertThat(config.getAttributeObject(LOG_LEVEL)).isEqualTo("config");
+
assertThat(config.getAttributeObject(SECURITY_LOG_LEVEL)).isEqualTo("config");
+ assertThat(config.getAttributeObject(REDUNDANCY_ZONE)).isEqualTo("");
+
assertThat(config.getAttributeObject(ENABLE_CLUSTER_CONFIGURATION).getClass())
+ .isEqualTo(Boolean.class);
}
@Test
public void testCheckerChecksValidAttribute() {
for (String att : checkers.keySet()) {
System.out.println("att = " + att);
- assertTrue(attributes.containsKey(att));
+ assertThat(attributes.containsKey(att)).isTrue();
Review comment:
Recommend:
```
assertThat(attributes).containsKey(att);
```
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
