Add more benchmarks, smoke tests, make FilterVar serializable#1607
Add more benchmarks, smoke tests, make FilterVar serializable#1607
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1607 +/- ##
============================================
+ Coverage 97.51% 97.78% +0.27%
- Complexity 1006 1011 +5
============================================
Files 212 213 +1
Lines 2334 2351 +17
============================================
+ Hits 2276 2299 +23
+ Misses 58 52 -6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
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
SmokeTestProvidertrait 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
FilterVarfrom extendingEnvelopeto directly implementingValidatorto 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.
|
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
left a comment
There was a problem hiding this comment.
My comments are mostly cosmetic, but the place where that test is, I think we should really change.
This commit introduces a new feature test: SerializableTest, that tests several validators for their ability to be serialized and unserialized. It also makes it so that the same list of validators can be used by both simple benchmarks and "smoke tests" of all kinds, including this serialize/unserialize one. Additionally, the FilterVar validator was modified. Previously, due to the use of Callback, it was not serializable, but now it is.
This commit introduces a new feature test: SerializableTest, that
tests several validators for their ability to be serialized and
unserialized.
It also makes it so that the same list of validators can be used
by both simple benchmarks and "smoke tests" of all kinds, including
this serialize/unserialize one.
Additionally, the FilterVar validator was modified. Previously, due
to the use of Callback, it was not serializable, but now it is.