Add more benchmarks, smoke tests, make FilterVar serializable #1607

alganet · 2026-01-16T04:59:16Z

This improves composer bench by adding a lot of new benchmarks.

The provider for benchmarks is now available as a generic smoke test provider that can be used when we want to test aspects of all validators. For example, serialize compatibility. A test for serialize compatibility was also introduced.

There are still a few validators that are not serializable. Image, Mimetype and Uploaded, mostly. Since those have some sort of similarity, they will be addressed later.

codecov · 2026-01-16T05:00:16Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.85%. Comparing base (8e5938a) to head (6969c14).

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1607      +/-   ##
============================================
+ Coverage     97.51%   97.85%   +0.34%     
- Complexity     1006     1007       +1     
============================================
  Files           212      212              
  Lines          2334     2335       +1     
============================================
+ Hits           2276     2285       +9     
+ Misses           58       50       -8

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

This PR enhances testing infrastructure by consolidating validator test data into a reusable trait, adding comprehensive benchmarks, and making the FilterVar validator serializable.

Changes:

Introduced SmokeTestProvider trait with comprehensive validator test data for reuse across benchmarks and serialization tests
Added serialization test coverage to verify validators can be serialized and deserialized
Refactored FilterVar from extending Envelope to directly implementing Validator to support serialization

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/library/SmokeTestProvider.php	New trait providing test data for all validators, enabling reuse across different test types
tests/unit/SerializableTest.php	New test verifying validators can be serialized/unserialized while maintaining functionality
tests/benchmark/ValidatorBench.php	Refactored to use `SmokeTestProvider`, expanding benchmark coverage from ~50 to ~150 validators
phpbench.json.dist	Updated bootstrap path to use tests/bootstrap.php for proper test environment setup
library/Validators/FilterVar.php	Refactored to implement `Validator` directly instead of extending `Envelope` for serialization support

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

library/Validators/FilterVar.php

henriquemoody · 2026-01-17T17:24:39Z

I would be great if you could add the information you have in the pull request to the commit message, it helps with adding more context when looking at the git logs.

Personally, I prefer when the commit tells what it does in the subject line, and explain why in the body. This subject line tells 3 different things, and it would be best to have a subject line that expresses (in imperative mood) the overarching subject of the commit.

henriquemoody

My comments are mostly cosmetic, but the place where that test is, I think we should really change.

henriquemoody · 2026-01-17T17:26:44Z

library/Validators/FilterVar.php

+    public function evaluate(mixed $input): Result
+    {
+        return Result::of(
+            (self::ALLOWED_FILTERS[$this->filter])(match ($this->options) {


Is match the new ternary? 😂

Not asking to change, I just thought it was an interesting way to use it.

Our combination of static analyzers doesn't allow multi-line ternaries, which I do in fact prefer, like this:

$isSomething ? whenTrue() : whenFalse()

It forces them to be inline, and then the line becomes too big for the line length rule. In this case, it doesn't justify creating methods to wrap the conditions.

A match circumvents that, and I used it as a hack definitely! It will lead to the same opcodes as a ternary though.

Not criticising, I genuinely found it interesting, and I can see the benefits

henriquemoody · 2026-01-17T17:28:51Z

tests/unit/SerializableTest.php

+    use SmokeTestProvider;
+
+    #[DataProvider('provideValidatorInput')]
+    public function testValidatorSerialization(Validator $v, mixed $input): void


Suggested change

public function testValidatorSerialization(Validator $v, mixed $input): void

[#Test]

public function validatorSerialization(Validator $v, mixed $input): void

I wish there was a rule for that in PHP_CodeSniffer

We might not even need to change that if we turn that into a feature test.

henriquemoody · 2026-01-17T17:29:47Z

library/Validators/FilterVar.php

    '{{subject}} must not be valid',
 )]
-final class FilterVar extends Envelope
+final class FilterVar implements Validator


Suggested change

final class FilterVar implements Validator

final readonly class FilterVar implements Validator

We can make it readonly now. I will look for a PHP_CodeSniffer rule that does that when possible.

henriquemoody · 2026-01-17T17:32:06Z

tests/unit/SerializableTest.php

+use function unserialize;
+
+#[Group('core')]
+final class SerializableTest extends TestCase


This is not a unit test, and although it's arguable not a feature test either, it's best to put it there, maybe turn into a Pest test?

Can I reuse the data provider in the Pest test cleanly? If you know how to do it, let me know.

You can use Datasets: https://pestphp.com/docs/datasets, maybe an extra array_map() around it, but about that

alganet requested a review from Copilot January 16, 2026 04:59

Copilot started reviewing on behalf of alganet January 16, 2026 04:59 View session

Copilot AI reviewed Jan 16, 2026

View reviewed changes

library/Validators/FilterVar.php Outdated Show resolved Hide resolved

Add more benchmarks, smoke tests, make FilterVar serializable

6969c14

alganet force-pushed the smoke-tests branch from c0c9905 to 6969c14 Compare January 16, 2026 05:07

alganet marked this pull request as ready for review January 16, 2026 05:08

alganet requested a review from henriquemoody January 16, 2026 05:08

henriquemoody reviewed Jan 17, 2026

View reviewed changes

	public function testValidatorSerialization(Validator $v, mixed $input): void
	[#Test]
	public function validatorSerialization(Validator $v, mixed $input): void

	final class FilterVar implements Validator
	final readonly class FilterVar implements Validator

Add more benchmarks, smoke tests, make FilterVar serializable #1607

Are you sure you want to change the base?

Add more benchmarks, smoke tests, make FilterVar serializable #1607

Conversation

alganet commented Jan 16, 2026

Uh oh!

codecov bot commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

henriquemoody commented Jan 17, 2026

Uh oh!

henriquemoody left a comment

Choose a reason for hiding this comment

Uh oh!

henriquemoody Jan 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Jan 16, 2026 •

edited

Loading

henriquemoody Jan 17, 2026 •

edited

Loading