https://www.drupal.org/pift-ci-job/838384 tests JSON API against PHP 7.1 + the next Drupal 8 minor (8.5 currently).

  • Nov 30: "299 pass"
  • Dec 1: "No tests found"
  • All daily test runs since then: "No tests found"

I dug into the DrupalCI console output. It contains this:

00:01:47.042 PHP Fatal error:  Class Drupal\Tests\jsonapi\Functional\RestJsonApiUnsupported contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (Drupal\Tests\rest\Functional\ResourceTestBase::getExpectedUnauthorizedAccessCacheability) in /var/www/html/modules/contrib/jsonapi/tests/src/Functional/RestJsonApiUnsupported.php on line 15
CommentFileSizeAuthor
#4 2931831-4.patch636 byteswim leers

Comments

Wim Leers created an issue. See original summary.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues:

#2765959: Make 4xx REST responses cacheable by (Dynamic) Page Cache + comprehensive cacheability test coverage introduced a new abstract method on a base class that this test is using. We need to update JSON API's test. This will work fine on Drupal 8.4 too.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#2765959: Make 4xx REST responses cacheable by (Dynamic) Page Cache + comprehensive cacheability test coverage
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Active » Needs review
StatusFileSize
new 2931831-4.patch636 bytes
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Reviewed & tested by the community

Apparently something changed in 8.5 that causes tests to fail. But since A) daily tests haven't been running, B) JSON API isn't tested against 8.5 in issues, we've been unaware!

Committing this fix, because it's better to be aware about a breakage than continuing to ignore it.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented 
+++ b/tests/src/Functional/RestJsonApiUnsupported.php
@@ -104,6 +104,11 @@ class RestJsonApiUnsupported extends ResourceTestBase {
+  protected function getExpectedUnauthorizedAccessCacheability() {}

You may wonder "why the hell do we have to implement this?"

The reason is that REST test coverage should think about this. It's just that this particular REST test doesn't need to care.

  • Wim Leers committed 17490c1 on 8.x-1.x 
    Issue #2931831 by Wim Leers: Tests not running against Drupal 8.5...
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Reviewed & tested by the community » Fixed
e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented

Thanks for taking care of this!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.