Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Write PHPUnit test to cover all classes.
Comment | File | Size | Author |
---|---|---|---|
#6 | interdiff-2771087-4-6.txt | 1.89 KB | juhog |
#6 | unit_tests-2771087-6.patch | 9.84 KB | juhog |
| |||
#4 | interdiff-2771087-3-4.txt | 1.85 KB | juhog |
#4 | unit_tests-2771087-4.patch | 9.94 KB | juhog |
| |||
#3 | unit_tests-2771087-3.patch | 9.85 KB | juhog |
|
Comments
Comment #2
beltofteComment #3
juhog CreditAttribution: juhog as a volunteer commentedI wrote some unit tests for the module. I hope this is the correct place to post the patch, eventhough the issue is closed. Apologies if this isn't the correct place.
Notes about the patch:
- I used Search API module's unit tests as examples and tried to apply the same ideas
- The tests cover only one method: alterIndexedItems()
- EntityManager removed, replaced by EntityFieldManager
- EntityFieldManager dependency is injected using a setter method
- Minor fix to bundleHasField() docblock (the return value type)
Comment #4
juhog CreditAttribution: juhog as a volunteer commentedThe test file had some coding standard issues. This patch fixes those.
Comment #5
borisson_I only have one meaningful thing to say about this patch. That is the last point in this review. The rest already looks super good. Great work!
Oh, it is good that you also fixed this in the meanwhile, it was wrong.
It's not strictly needed to have both the @see and the @coversDefaultClass point at the same processor, on the other hand, it doesn't hurt either.
We can change this with assertEmpty/assertNotEmpty to be clearer.
Comment #6
juhog CreditAttribution: juhog as a volunteer commentedThanks for the comments!
Here's a new patch with the following changes:
Comment #7
borisson_Yeah, that also works! Thank @juhog!
Comment #9
beltofteCommitted!
Thanks @juhog and @borisson_!