search

TNG / ArchUnitNET

A C# architecture test library to specify and assert architecture rules in C# for automated testing.
Apache License 2.0
826 stars 55 forks source link

Require positive results by default #201

Closed simonthum closed 8 months ago

simonthum commented 1 year ago

This is a breaking change that requires rules to yield positive results by default. The intent is to protect against misformulated rules.

This is the first public iteration. It still loses type information on WithoutRequiringPositiveResults(), but otherwise I think it's fine.

Resolves #189

simonthum commented 1 year ago

The new code properly highlights the fact that the example tests do not test anything:

[xUnit.net 00:00:02.13]     ExampleTest.ExampleArchUnitTest.ExampleClassesShouldNotCallForbiddenMethods [FAIL]
  Failed ExampleTest.ExampleArchUnitTest.ExampleLayerShouldNotAccessForbiddenLayer [566 ms]
  Error Message:
   ArchUnitNET.xUnit.FailedArchRuleException : "Types that are Example Layer should not depend on Forbidden Layer because it's forbidden" failed:
    The rule requires positive evaluation, not just absence of violations. Use WithoutRequiringPositiveResults() or improve your rule's predicates.

Although something needs to be done about it, I view this as an encouraging sign.

I'll include the signoffs.

fgather commented 1 year ago

Hey @simonthum , thanks for the PR, it looks really promising. @MSinstein , @fholger , what do you think?

fholger commented 1 year ago

Hey @simonthum , thanks for the PR, it looks really promising. @MSinstein , @fholger , what do you think?

Looks good to me, though it is a breaking change and would probably benefit from some beta-testing by other real-world users.

simonthum commented 1 year ago

I agree with the concern. Is there an update site for CI results or qualified nuget builds available?

fgather commented 1 year ago

I will try to public some beta-release on nuget

simonthum commented 1 year ago

Hey, is there any progress to be aware of?

WietseDeKoninckPXL commented 1 year ago

This is an absolutely required addition (in this form or another) in my opinion.

As a new user I was confused that rules still passed even though absolutely nothing was modeled in my architecture yet, which means

A) There were technically no violations B) But also no reason that warrants the existence of the rule

So if a user writes a rule that doesn't apply to the architecture nor presents any violations, the rule does not test anything.

simonthum commented 8 months ago

Thanks for merging!

However, it probably makes sense for a maintainer to stop by and look at the loss of type information I mentioned.