Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Empty rule verification should not apply to noMethods/noClasses #806

Closed
pkubowicz opened this issue Feb 21, 2022 · 3 comments
Closed

Empty rule verification should not apply to noMethods/noClasses #806

pkubowicz opened this issue Feb 21, 2022 · 3 comments

Comments

@pkubowicz
Copy link
Contributor

pkubowicz commented Feb 21, 2022

#774 should not be executed for rules written using noMethods/noClasses. Otherwise those methods make no sense: I want to guard against illegal code appearing in my codebase. If there is no code matching what I the rule defines, it's a success, not an error.

Imagine I have a rule checking I don't write broken JUnit 5 tests:

@AnalyzeClasses(packages = ["com.example"])
class ArchitectureTest {
    @ArchTest
    val noVoidTestFactory = noMethods().that().areAnnotatedWith("org.junit.jupiter.api.TestFactory")
        .should().haveRawReturnType("void")
}

Now some bad code:

class SomeTest {
    @Test
    fun helloWorld() {
        println("ok")
    }

    @TestFactory
    fun wrong() {
        println("I will be never executed")
    }
}

The architecture test detects this bad code. But now fix the bad test: remove wrong method together with its annotation. The architecture test will start to fail (with com.tngtech.archunit:archunit-junit5:0.23.0):

Rule failed to check any classes. This means either that no classes have been passed to the rule at all, or that no classes passed to the rule matched the `that()` clause. To allow rules being evaluated without checking any classes you can set the ArchUnit property archRule.failOnEmptyShould = false

This is a regression in 0.23.0. The architecture rule worked correctly in 0.22.0.

@pkubowicz pkubowicz changed the title Empty rule verification should not apply to noMethod/noClass Empty rule verification should not apply to noMethods/noClasses Feb 21, 2022
@codecholeric
Copy link
Collaborator

I understand your issue, I guess this is similar to #808. We probably have to make this overridable per rule.
I would not turn it off for noClasses() in general though, because it does make sense in many cases. Take noClasses().that().resideInPackage("doesNotExist").should().... This rule still was likely not intended this way and we should warn the user.
This is also a matter of how you write your rules. You could write your example as

noMethods().should(beAnnotatedWith("org.junit.jupiter.api.TestFactory").and(haveRawReturnType("void")))

and it would work as expected even with failOnEmptyShould = true 🤔

For now to make your rule work as before you can simply put an archunit.properties file in the root of your classpath and set archRule.failOnEmptyShould = false as the message suggests. Then your rule should work as before again (compare https://www.archunit.org/userguide/html/000_Index.html#_fail_rules_on_empty_should).

Unfortunately this will always be a trade-off. Before there have been many issues about people requesting a way to verify that their rules really test something (e.g. if you refactor the package of your main code). On the other hand this can now bring some inconvenience for cases where it is justified to not have any should-input.

@pkubowicz
Copy link
Contributor Author

This is a nice example of how a rule using noMethods can be rewritten to work well with the new feature. Maybe you can add it to the user manual?

@pkubowicz
Copy link
Contributor Author

I'm closing this issue as suggestion of rewriting my rules from #806 (comment) worked for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants