Architecture rule validation: expect passes

[simonthum]

Hi,

I would like to ask about a rule validation feature I came up with.

Most of my rules are written so that they validate types. However I managed to write some that did not fail on my architecture because they did not match any type before checks ran, due to a typo.

I can identify this using ...Should().Exist().AndShould()..., but preferred to write my own check routine to avoid repeating that clause.

In fact I think this should be the default checking behaviour and implemented it for XUnit:

        public static void CheckRule(Architecture architecture, IArchRule archRule, bool expectPasses = true)
        {
            bool hasPasses = false, hasFailures = false;

            foreach (var evaluationResult in archRule.Evaluate(architecture))
            {
                hasPasses |= evaluationResult.Passed;
                hasFailures |= !evaluationResult.Passed;

                if (hasFailures)
                    break;
            }

            if (hasFailures)
            {
                throw new FailedArchRuleException(architecture, archRule);
            }
            if (expectPasses && !hasPasses)
            {
                throw new XunitException("An architecture rule did not produce any passes, please specify expectPasses to be false if that is fine.");
            }
        }

The effect is similar to ...Should().Exist().AndShould()... early in the rule, but is on by default.

That way it is a breaking change, but I think the added validation is worth it. What do people think?

I'd write the code for all Unit Test frameworks if consensus is reached.

[fgather]

@simonthum , since I had a very similar issue once, I think the idea is great. ArchUnit for Java has a very similar feature: https://github.com/TNG/ArchUnit/pull/774. They implemented it into the ArchUnit core and not into the TestRunner-Code, which I think makes sense from a consistency point of view.

However, there are use-cases, where an empty Input-set is needed (see: https://github.com/TNG/ArchUnit/pull/816).

I do not see an issue though to find a similar solution for the issue. Do you have a proposal for the Syntax?

[simonthum]

Sorry for the late response!

I'd suggest either the syntax implied above as the new default:

arch.CheckRule(myRule) // compiler defaults to expectPasses = true

or something like

myClasses.AllowEmpty().Should()...

with a missing AllowEmpty() call defaulting to check against emptiness.

[simonthum]

I have a simple thing running now, based on a modified Should(bool allowEmptiness).

As expected, it makes a bunch of tests fail (about 30). This particular rule highlights the missing improvements:

Types().That().Are(typeof(ClassWithMultipleAttributesWithParameters)).And()
    .HaveAnyAttributesWithNamedArguments(("Parameter2", "param1_1"))
    .Should().NotExist()
    .Check(Architecture);

To handle this as one would expect the check needs to be deferred and be made conditional.

The current approach requires the user to modify the rule:

.Should(allowEmptiness: true)

The question is how to proceed. Do you consider that state of affairs acceptable or should I try to defer?

[simonthum]

I put in some more effort to defer evaluation. Exist/NotExist seems fine now.

It no longer checks for empty input, but for positive results, which should be mostly identical and avoids clashes with the internal Empty() logic required for Not/Exists.