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

Enforce test failures based on the priority set #1337

Open
arundotin opened this issue Aug 2, 2024 · 2 comments
Open

Enforce test failures based on the priority set #1337

arundotin opened this issue Aug 2, 2024 · 2 comments

Comments

@arundotin
Copy link

arundotin commented Aug 2, 2024

Let's say that I have a rule this way

ArchRule rule = priority(Priority.LOW)
                .classes()
                .that()
                .resideInAPackage("com.example.utils")
                .should()
                .haveSimpleNameEndingWith("Util")
                .because("utility classes should have a 'Util' suffix");

        rule.check(classes);

I should be able to set somewhere that even if this rule is violated I should simply throw a warning, but not fail the test suite.

How do I do that ? There should be some native way to do this, as opposed to doing it via a huge method.

if there's no native way, I guess that could be prioritized :)

@hankem
Copy link
Member

hankem commented Aug 2, 2024

  1. I think that for ArchUnit's internal rules, rule.check(classes) currently always ends up in ArchRule.Assertions.check(rule, classes), which throws an AssertionError on failures, regardless of the priority , which should be available in the EvaluationResult and FailureReport.
  2. What can a test reasonably do on violations other than fail? Log the report and hope that somebody will read it? (Good luck!)
    • If you're willing to wrap all your rules manually, you could easily define an ArchRule decorator for such a behavior:
      public static ArchRule justLogFailuresOfPrioLowRules(ArchRule rule) {
          return new ArchRule() {
              @Override
              public void check(JavaClasses classes) {
                  EvaluationResult result = rule.evaluate(classes);
                  if (result.getPriority() != Priority.LOW) {
                      assertNoViolation(result);
                  } else {
                      FailureReport failureReport = result.getFailureReport();
                      if (!failureReport.isEmpty()) {
                          System.out.println(failureReport);
                      }
                  }
              }
      
              @Override
              public ArchRule because(String reason) {
                  return justLogFailuresOfPrioLowRules(rule.because(reason));
              }
      
              @Override
              public ArchRule allowEmptyShould(boolean allowEmptyShould) {
                  return justLogFailuresOfPrioLowRules(rule.allowEmptyShould(allowEmptyShould));
              }
      
              @Override
              public ArchRule as(String descirption) {
                  return justLogFailuresOfPrioLowRules(rule.as(descirption));
              }
      
              @Override
              public EvaluationResult evaluate(JavaClasses classes) {
                  return rule.evaluate(classes);
              }
      
              @Override
              public String getDescription() {
                  return rule.getDescription();
              }
          };
      }
    • If you want to automatically apply that behavior for every rule, you could probably look into a custom Junit TestEngine. FYI: For ArchUnit's JUnit 5 support, the rule evaluation happens in com.tngtech.archunit.junit.internal.ArchUnitTestDescriptor.ArchUnitRuleDescriptor.

@codecholeric
Copy link
Collaborator

To be honest, I originally did have in mind to configure different behavior based on the priority 😁 But somehow nobody ever requested anything and I wasn't completely sure how this should look like in practice to be really useful. Because over the years I also again and again saw situations where some stuff like this would be logged as a warning and I haven't seen once that dev teams really took action on this. The only way such things were ever actually improved then was to fail the build on these warnings.
Just out of curiosity, if it was possible to just log this as a warning, how would you expect this to be dealt with? Maybe I'm just lacking the context of how you want to use this in your daily workflow.

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

3 participants