EntityResource: Provide comprehensive test coverage for BlockContentType entity

I can't make it work.

I can't even run a previous working test. For example :

php core/scripts/run-tests.sh --url http://drupalvm.dev  --verbose --class "Drupal\Tests\rest\Functional\EntityResource\Block\BlockJsonAnonTest"

returns

    Drupal\Tests\rest\Functional\EntityResource\Block\BlockJsonAnonTest::testGet
    Failed asserting that Array &0 (
        0 => 'https://drupal.org/link-relations/delete-form'
    ) is identical to Array &0 (
        0 => 'https://drupal.org/link-relations/delete-form'
        1 => 'edit-form'
        2 => 'https://drupal.org/link-relations/enable'
        3 => 'https://drupal.org/link-relations/disable'
    ).

with a clean drupalvm install. Is it normal ?

Thanks for the reply !

I've tried what Wim Leers said in your post, but I have the exact same output :

vagrant@drupalvmRest:/var/www/drupalvm/drupal/web$ php vendor/bin/phpunit -c core core/modules/rest/tests/src/Functional/EntityResource/Block/
PHPUnit 4.8.35 by Sebastian Bergmann and contributors.

F...F...F...

Time: 2.54 minutes, Memory: 6.00Mb

There were 3 failures:

1) Drupal\Tests\rest\Functional\EntityResource\Block\BlockJsonAnonTest::testGet
Failed asserting that Array &0 (
    0 => 'https://drupal.org/link-relations/delete-form'
) is identical to Array &0 (
    0 => 'https://drupal.org/link-relations/delete-form'
    1 => 'edit-form'
    2 => 'https://drupal.org/link-relations/enable'
    3 => 'https://drupal.org/link-relations/disable'
).

/var/www/drupalvm/drupal/web/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php:440

2) Drupal\Tests\rest\Functional\EntityResource\Block\BlockJsonBasicAuthTest::testGet
Failed asserting that Array &0 (
    0 => 'https://drupal.org/link-relations/delete-form'
) is identical to Array &0 (
    0 => 'https://drupal.org/link-relations/delete-form'
    1 => 'edit-form'
    2 => 'https://drupal.org/link-relations/enable'
    3 => 'https://drupal.org/link-relations/disable'
).

/var/www/drupalvm/drupal/web/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php:440

3) Drupal\Tests\rest\Functional\EntityResource\Block\BlockJsonCookieTest::testGet
Failed asserting that Array &0 (
    0 => 'https://drupal.org/link-relations/delete-form'
) is identical to Array &0 (
    0 => 'https://drupal.org/link-relations/delete-form'
    1 => 'edit-form'
    2 => 'https://drupal.org/link-relations/enable'
    3 => 'https://drupal.org/link-relations/disable'
).

Works fine here:

wim.leers at acquia in ~/Work/d8 on 8.4.x*
$ php vendor/bin/phpunit -c core core/modules/rest/tests/src/Functional/EntityResource/Block/BlockJsonAnonTest.php
PHPUnit 4.8.27 by Sebastian Bergmann and contributors.

....

Time: 17.04 seconds, Memory: 6.00Mb

OK (4 tests, 57 assertions)

Something must be wrong with your environment setup.

It's possible, as ever, that I've missed something here, but when I run the functional block tests (both for the HAL module and the REST module) I get twelve passes each time:

vagrant:///Users/me/VagrantBoxes/vdd/usr/bin/php /home/vagrant/.phpstorm_helpers/phpunit.php --configuration /var/www/drupal8/core/phpunit.xml /vagrant/data/drupal8/core/modules/hal/tests/src/Functional/EntityResource/Block
Testing started at 17:03 ...
PHPUnit 4.8.35 by Sebastian Bergmann and contributors.

Time: 3.25 minutes, Memory: 6.75MB

OK (12 tests, 176 assertions)

Process finished with exit code 0

vagrant:///Users/me/VagrantBoxes/vdd/usr/bin/php /home/vagrant/.phpstorm_helpers/phpunit.php --configuration /var/www/drupal8/core/phpunit.xml /vagrant/data/drupal8/core/modules/rest/tests/src/Functional/EntityResource/Block
Testing started at 17:10 ...
PHPUnit 4.8.35 by Sebastian Bergmann and contributors.

Time: 3.09 minutes, Memory: 6.50MB

OK (12 tests, 176 assertions)

Process finished with exit code 0

Are we actually referring to the data available at the /entity/block_content_type/{block_content_type} endpoint?

So the the endpoint to get the default-installed Basic Block would be /entity/block_content_type/basic?_format=json and the entity returned would look like this:

{
uuid: "9b60112e-15d5-42c7-ad03-81910c3211d5",
langcode: "en",
status: true,
dependencies: [ ],
_core: {
default_config_hash: "zglzjmYxi0G0ag9MZ02y0LSJOdpWRwJxyP_OvFojFyo"
},
id: "basic",
label: "Basic block",
revision: 0,
description: "A basic block contains a title and a body."
}

Which I'm guessing is what should be returned by the getNormalizedEntity function in a BlockContentTypeResourceTestBase class?

I'm going to assign this to myself if that's cool with everyone?

Yep, thanks for taking this on, @jamesdesq! :)

@vaplas - thanks - as always, for your help - that's really really useful.

It's been a bank holiday weekend here in the UK so I've mostly been in the pub, but I've got some time today so with any luck I'll have a first try up in a couple of hours.

OK, so one question (sorry, always with the questions!)...

I'm not sure I fully understand what you mean in #12 when you say the log will contain what should be returned.

I've got the REST tests up and running (actually 3/4 of them are redundant, since the block_content_type endpoint only implements get, but it looks as though those tests pass - I'm assuming since it's an instance of ConfigEntityInterface and so gets caught by the first condition in testPost, testPatch and testDelete).

The first part of my BlockContentTypeResourceTestBase looks like this. I've tried explicitly logging the $block_content_type variable, but nothing seems to show up in the logs. Is that what you meant, or have I misunderstood? (FTR, I also tried returning an empty array from the getExpectedNormalizedEntity, which is what I initially thought you meant).

<?php

namespace Drupal\Tests\rest\Functional\EntityResource\BlockContentType;

use Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase;
use Drupal\block_content\Entity\BlockContentType;

abstract class BlockContentTypeResourceTestBase extends EntityResourceTestBase {

  /**
   * {@inheritdoc}
   */
  public static $modules = [
    'block',
    'block_content'
  ];

  /**
   * {@inheritdoc}
   */
  protected static $entityTypeId = 'block_content_type';

  protected $entity;

  /**
   * {@inheritdoc}
   */
  protected function setUpAuthorization($method) {
    $this->grantPermissionsToTestedRole(['administer blocks']);
  }

  protected function createEntity() {
    $block_content_type = BlockContentType::create([
      'id' => 'pascal',
      'label' => 'Pascal',
      'revision' => FALSE,
      'description' => "Provides a competitive alternative to the 'basic' type",
    ]);

    $block_content_type->save();

    \Drupal::logger('rest_test')->notice($block_content_type);

    return $block_content_type;

  }

  protected function getExpectedNormalizedEntity() {
    // return array containing what should be returned

  }

@jamesdesq By the way, feel free to post imperfect, in-progress patches. We all do that. It helps others to see how far along this is, and provide suggested changes to help you move quicker :)

Thanks both. This is all super useful stuff to know.

So, this seems to pass locally for me. Could someone else have a try please?

Like this?

Also, who is Mr Bot? And how do I get him to run my tests?

Sorry , i didn't see issue is assigned to @jamesdesq.Setting it at needs review will get Mr Bot to run tests.

Forgot to interdiff, sorry...

Thanks for your patch, @gaurav.kapoor! I think yours and mine are just about the same - but it doesn't look as though you deleted the unnecessary uuid variable in the BlockContentTypeResourceTestBase.

Testbot reports one coding standards violation in its test results. Could you please fix that?

Other than that detail, this is ready!

Here it is with the coding violation corrected (I'd do an interdiff but I kind of figure for a single newline it's probably not worth it).

Thanks for the explanation, @vaplas!

@jamesdesq Can you please provide an interdiff? See https://www.drupal.org/documentation/git/interdiff.

Should the interiff be between #31 and #23? Or between #31 and #18? I kind of felt like the difference between #31 and #23 was so small it wasn't really worth it, but I'm happy to do it. Or am I missing the point?

#34: the point of interdiffs is to make it easier for reviewers. Right now, I have to re-read the entire patch, because I don't know which changes you made. Perhaps you changed only what was asked for, perhaps you changed more. An interdiff answers that with much more clarity: it shows exactly what changed between patches.
We all make mistakes. Interdiffs help catch those mistakes earlier :)

Those are all good reasons :)

Here's the interdiiff!

Thank you!

#39: hah, well-spotted! :) I'm impressed by how fast you became so proficient in rolling patches, doing reviews and doing mentoring, @vaplas!

So the final version should look like this, I think...

Once again, thanks both for all the help and encouragement on this, it's been great (though I think I'm a way of Jedi-ness just yet). I'm hoping to get some more tickets from this group done over the next few days!

Sorry, I thought I'd uploaded the interdiff but it didn't seem to work

I'm hoping to get some more tickets from this group done over the next few days!

Woot! Still plenty to go in #2824572: Write EntityResourceTestBase subclasses for every other entity type. :)

This now looks ready again.

Committed and pushed f170a96 to 8.4.x and 4511622 to 8.3.x. Thanks!

I've backported to 8.3.x because it is tests only.

I credited @gaurav.kapoor because they supplied a patch which for some reason d.o doesn't see. @Keenegan thanks for trying to review and contribute to the issue. I'm hope you've got phpunit set up and working now.