nastra commented on code in PR #7131:
URL: https://github.com/apache/iceberg/pull/7131#discussion_r1141738191
##########
CONTRIBUTING.md:
##########
@@ -194,3 +194,83 @@ When passing boolean arguments to existing or external
methods, use inline comme
* For example, preferred convection `access-key-id` rather than
`access.key.id`
2. Use `.` to create a hierarchy of config groups
* For example, `s3` in `s3.access-key-id`, `s3.secret-access-key`
+
+## Testing
+
+### AssertJ
+
+Prefer using [AssertJ](https://github.com/assertj/assertj) assertions as those
provide a rich and intuitive set of strongly-typed assertions.
+Checks can be expressed in a fluent way and
[AssertJ](https://github.com/assertj/assertj) provides rich context when
assertions fail.
+Additionally, [AssertJ](https://github.com/assertj/assertj) has powerful
testing capabilities on collections and exceptions.
+Please refer to the [usage
guide](https://assertj.github.io/doc/#assertj-core-assertions-guide) for
additional examples.
+
+```java
+// bad: will only say true != false when check fails
+assertTrue(x instanceof Xyz);
+
+// better: will show type of x when check fails
+assertThat(x).isInstanceOf(Xyz.class);
+
+// bad: will only say true != false when check fails
+assertTrue(catalog.listNamespaces().containsAll(expected));
+
+// better: will show content of expected and of catalog.listNamespaces() if
check fails
+assertThat(catalog.listNamespaces()).containsAll(expected);
+```
+```java
+// ok
+assertNotNull(metadataFileLocations);
+assertEquals(metadataFileLocations.size(), 4);
+
+// better: will show the content of metadataFileLocations if check fails
+assertThat(metadataFileLocations).isNotNull().hasSize(4);
+
+// or
+assertThat(metadataFileLocations).isNotNull().hasSameSizeAs(expected).hasSize(4);
+```
+
+```java
+// bad
+try {
+ catalog.createNamespace(deniedNamespace);
+ Assert.fail("this should fail");
+} catch (Exception e) {
+ assertEquals(AccessDeniedException.class, e.getClass());
+ assertEquals("access denied", e.getMessage());
+}
+
+// better
+assertThatThrownBy(() -> catalog.createNamespace(deniedNamespace))
+ .isInstanceOf(AccessDeniedException.class)
+ .hasMessage("access denied");
Review Comment:
makes sense, done
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
