-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor some mapping tests for improved coverage and forward compat #2732
Refactor some mapping tests for improved coverage and forward compat #2732
Conversation
b3b1f57
to
4ec7bde
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2732 +/- ##
==========================================
+ Coverage 79.27% 79.40% +0.13%
==========================================
Files 162 162
Lines 8467 8494 +27
==========================================
+ Hits 6712 6745 +33
+ Misses 1755 1749 -6 ☔ View full report in Codecov by Sentry. |
@@ -570,6 +570,11 @@ parameters: | |||
count: 3 | |||
path: src/Uploadable/UploadableListener.php | |||
|
|||
- | |||
message: "#^Class Gedmo\\\\Tests\\\\Translatable\\\\Fixture\\\\CategoryTranslation not found\\.$#" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Baselining this because the YAML mapping that the annotation/attribute mappings were generated from references this non-existing class, and PHPStan now (rightfully) groans about it. The test fixtures should probably use an existing class at some point in the future, but for now, its absence doesn't break anything.
4ec7bde
to
f5224ba
Compare
thanks @mbabker! |
Ref: #2708
Right now there are a lot of tests that rely on the deprecated annotation and YAML drivers from the ORM to actually test anything. This means in a lot of cases the XML mapping isn't explicitly tested in a mapping case (it is incidentally covered elsewhere), nor can the tests run without the deprecated Annotations package. This is a first step at improving some of the test cases to be able to cover XML and explicitly test attribute mapping (and while all the extension-level attribute drivers are just an extension of their respective annotation drivers, there should be coverage when working purely in the context of attributes and not using the annotations package at all).
This PR refactors the loggable, sluggable, and soft-deleteable mapping tests to conditionally test mapping drivers based on whether the underlying dependencies are available so that the tests automatically skip themselves when needed while also supporting covering all mapping options. I'm mostly building on and re-using existing test fixtures where they exist, and adding attributes where they don't exist on annotated classes, so some stuff might not exist at all right now (i.e. there isn't an XML mapping or fixture class for the
Category
models tested with other drivers). Following this pattern for at least the mapping tests, this will help ensure explicit coverage on all drivers while only testing drivers that the environment supports.