Add Architecture Testing

[aidan-casey]

@vsergiu93 brought up the idea of architecture testing in #114. I think this is a great idea and something work exploring.

Looking at ta-tikoma/phpunit-architecture-test (which Pest uses) it does not appear they currently have support for attributes. It seems like that would be a pretty key thing considering discovery and validation rules.

There are some alternatives (phpat, etc.) that do support attributes and are probably worth digging into further.

[Plytas]

What about adding Pest? All main tests can still be written using phpunit syntax, but architecture tests could be written using pest which seems to have attribute support.

[brendt]

So, let's talk about Pest. In a way it would be fitting to use it as the default test runner for Tempest. Pest emerged as a way to break with "the standard", and took a whole different approach to testing.

Personally, I'm not for or against it, although it would require some time to get used to it.

On top of that, I see a small "marketing" opportunity for Tempest if we embrace Pest as the default test runner.

And even better, "Tempest", I mean, speaking of a match made in heaven :p

[aidan-casey]

Personally, I'm not a Pest guy, for no other reason than I prefer the syntax and structure of PHPunit.

As far as what decision gets made here goes, I'd just caution against making that decision based on functionality like architecture testing, which is essentially a wrapper for another package and primarily weigh the pros (or cons) of Pest's style.

[Plytas]

Since Pest is progressive framework it can run on top of phpunit syntax. There's no need to migrate existing tests or even write new tests using pest syntax. Heck, people could write with whichever syntax they prefer and it would still work, with the downside being that tests now have mixed structure and can be confusing.

Because of that, at work we opted to continue writing tests using phpunit syntax as everyone was familiar with it. We did however add pest for the sake of architecture testing. A single file that contains some basic rules to make sure everyone adheres to them.

I understand the argument that we could use the underlying package and implement it ourselves, but at this point it would feel like re-inventing the wheel when there's already a working solution available.

Also, Pest comes with parallel testing, profiling slow tests, test coverage and type coverage out of the box.

Though I am curious if parallel testing would work properly when tests hit the database. Laravel has solved it and you can run tests using php artisan test --parallel.

[aidan-casey]

In some sense, having two different syntaxes would be a concern, in my mind. Whatever we choose, it should be consistent.

Parallel testing, architecture testing, etc. are all literally PHPunit extensions. There is no reinventing the wheel, you just install the packages and go. 🙂

[brendt]

How about I'll implement the PHPUnit architecture plugin, and if someone want to make the pest equivalent, we can compare?

[Plytas]

I think for this to work there will need to be some restructuring in the code base.

Let's take Disvocery for example:

Using pest it would be possible to write this kind of test

But currently this would fail because that namespace also includes Discovery interface and DiscoveryLocation class. If I move those 2 classes somewhere else, then the test passes with the exception that MigrationDiscovery is not readonly.

[aidan-casey]

We are planning to restructure those anyway, but you can technically add the classes() modifier here. 🙂

I think, initially anyway, the things that are valuable to architecture test will be somewhat niche and then broaden with time. For example, we might start with: are validation rule classes attributes?

[Plytas]

I've added an example in https://github.com/tempestphp/tempest-framework/pull/141

[Plytas]

Have you guys had any more thoughts on this? I saw Brent made a Twitter poll, and it seems to be heavily skewed towards Pest. Quite expected, I would say.

If you're still unsure about Pest, I would like you to consider a 3rd approach. Using Pest for architecture tests, but keeping PHPUnit syntax for all the other tests. I know it may seem that it breaks consistency, but architecture tests should be a "write once" thing that shouldn't change unless you need to test new concepts. Other framework tests could still be written using PHPUnit. IMO this separation could be a valid excuse for inconsistency.

[aidan-casey]

Have you guys had any more thoughts on this?

Not yet. Since this will effect (hopefully) several years of future work for us, we want to make sure we make the right choice in the long run. Ultimately this will be up to @brendt as our benevolent dictator. 😉

I saw Brent made a Twitter poll, and it seems to be heavily skewed towards Pest. Quite expected, I would say.

I think it's worth pointing out that Brent's audience is predominantly a Laravel one that has a significant bias toward Pest. Despite that, I'm not sure that necessarily represents well the majority of our contributors and is for sure something we need to keep into account.

Using Pest for architecture tests, but keeping PHPUnit syntax for all the other tests.

I'll let @brendt speak to this, but I think my primary concern would be the confusion that might cause for contributors. 🙂

[brendt]

First, thank you @Plytas for the effort put into this, really appreciate it!

I gave this a lot of thought. Right now, I'm leaning more towards PHPAT.

There are three reasons:

• I think architecture testing leans conceptually closer to static analysis instead of runtime testing.
• Pest is for many developers outside the Laravel community very much unknown.
• If we go with Pest, I think we should embrace it in full, and in light of where we're at with Tempest, that's something I don't want to deal with right now

I want to leave the option open in the future to switch to Pest, but right now PHPAT seems like the more reasonable thing to do. Before any final decisions, I want to check with @Plytas and @aidan-casey if you have any followup thoughts?

Small sidenote btw on that Twitter poll: my audience is heavily skewed towards Laravel, and definitely doesn't give an accurate representation of PHP as a whole. Looking at, for example, anonymous PhpStorm usage stats, Pest is definitely a minority amongst the broader community.

[Plytas]

That's exactly what I meant about twitter poll being skewed when saying "Quite expected" considering your audience. Didn't mean it as an indicator that Pest is preferred one.

I think your arguments for PHPAT are completely valid. Once decision is finalized I'd be able to implement more rules. Would be helpful if you would have a list of rules you already have or can foresee.

[aidan-casey]

I think that all makes sense. I appreciate your continued efforts here, @Plytas!

[aidan-casey]

@Plytas - I'd like to think about some rules to put in place. It makes sense to come up with a list. There are probably some things we can do with initializers and attributes.

If you have suggestions, I am completely open to hearing them as well.

[brendt]

I'm currently deep in some other parts of the framework, so I'll come back to this soon. Definitely go ahead and propose some more rules 👍

A "general" ruleset that should be used everywhere where possible:

• Never use abstract classes
• Classes that implement any interface should be final readonly
• Classes that don't implement should be final but not always readonly (most of the time these are data objects, sometimes they have mutable data)

There are probably exceptions to these rules though 😅