I am using log4j2 and logging the error as such:
String param1 = "param1";
try {
...
} catch (Exception e) {
log.error("This is an error: {}", param1, e);
}
This is my ArchUnit test, which tests that all the log.error calls contain throwable parameter:
void errors_should_be_logged_with_context() {
ArchRule rule =
noClasses()
.should(
callMethodWhere(
target(name("error"))
.and(target(owner(assignableTo(Logger.class))))
.and(
target(
rawParameterTypes(
new DescribedPredicate<>(
"logger.error without exception context") {
@Override
public boolean test(List<JavaClass> methodParameters) {
boolean hasNoContext = methodParameters.size() <= 1;
boolean hasNoExceptionInContext =
methodParameters.stream()
.noneMatch(
j -> j.isAssignableTo(Throwable.class));
return hasNoContext || hasNoExceptionInContext;
}
}))))
.as("use logger.error without context"));
rule.check(ALL_CLASSES);
}
The problem is that j.isAssignableTo(Throwable.class))
is returning false for the parameter at index 2.
Test works as expected if there are only 2 parameters in log.error call (string, throwable).
How should the test be modified to cover both scenarios?
You're testing the types of the parameters, not the type of the arguments.
Logger.error(String, Throwable)
exists as a method, but for more args than that, you're using plain Objects.
e.g. Logger.error(String, Object, Object)
. Beyond 10 parameters, there's a varargs Logger.error(String, Object...)
too.
The use of rawParameterTypes
is wrong, but I'm not familiar enough with ArchUnit's API to say what to replace it with.