ContentEntityBase::set() does not respect its interface

Awesome that you wrote a test, thanks!

I looked at how the rest of the tests in that class are implemented and I noticed that we can simplify the test I think.

+++ b/core/tests/Drupal/Tests/Core/Entity/ContentEntityBaseUnitTest.php
@@ -524,4 +524,32 @@ public function testGetFields($expected, $include_computed, $is_computed, $field
+    //Mock ContentEntityBase
+    $mock_base = $this->getMockBuilder('Drupal\Core\Entity\ContentEntityBase')
+      ->disableOriginalConstructor()
+      ->setMethods(array('get', 'getTranslatedField'))
+      ->getMockForAbstractClass();
+
+    //Mock FieldItemInterface
+    $mock_field = $this->getMockBuilder('Drupal\Core\Field\FieldItemInterface')
+      ->disableOriginalConstructor()
+      ->setMethods(array('setValue'))
+      ->getMockForAbstractClass();
+
+    //Set up expectations for get(), it returns the mocked FieldItemInterface
+    $mock_base->expects($this->any())
+      ->method('get')
+      ->willReturn($mock_field);
+
+    //Exercise set(), check if it returns $this
+    $this->assertSame(
+      $mock_base,
+      $mock_base->set(null, null)
+    );

ContentEntityBaseUnitTest already has a $this->entity which gets prepared in setUp() which you can use directly.

So the test could be written as

$this->assertSame($this->entity, $this->entity->set('id'));

or similar.

Yes, you are completely right. Sorry, I thought I had seen some existing usage of $this->entity->set() but that was in a different test class. Sorry, stupid me.

ContentEntityBaseUnitTest is already pretty weird in that it's more of an integration test, but I still think it makes sense to align this more with the current code. I.e. in theory I think your code is preferred (because it's an actual unit test) but it's not inline with the rest of the class.

So I played around a bit and the following seems to work:
Add

    $this->fieldTypePluginManager->expects($this->any())
      ->method('createFieldItemList')
      ->will($this->returnValue($this->getMock('Drupal\Core\Field\FieldItemListInterface')));

in setUp() where $this->fieldTypePluginManager is being set up.

Then my above sample code should work (but for the fact that I forgot the second required argument).

Can you try that @banviktor?

Removing the Novice tag, this is getting a bit convoluted... :-)

Awesome, this looks great to me.

+    //Exercise set(), check if it returns $this

@banvikor: Our coding standards dictate that there's an empty space between the "//" and the begining of the sentence.

I'm hoping that can be fixed on commit, so marking RTBC.

Awesome work @banviktor, welcome to the team! :-)