Replace, in tests, mocks that do not configure doubles with their actual objects

There are 24 uses of ->onlyMethods([]) following the parent issue, I think we should investigate them all here as most of them seem to be mocking the object under test, when we should probably just instantiate a real object.

Unsure if this is a step too far, but ContextualLinkManagerTest sets up a bunch of mocks and injects them into the main object with reflection and sets onlyMethods([]) - we can avoid all that simply by using the real constructor. The plugin factory doesn't even need mocking at all after this.

Similarly, when testing form validation we mock a FormValidator but then just pass in some arguments and don't stub out any methods, so we might as well use the real validator.

Two breadcrumb builders also can have similar treatment.

The other cases are where we mock abstract classes, or call disableOriginalConstructor() in unit tests, so these seemingly do need to stay as mocks.

All the changed code looks good to me.
Do we want to do more changes?

+++ b/core/tests/Drupal/Tests/Core/Menu/ContextualLinkManagerTest.php
@@ -73,59 +60,34 @@ class ContextualLinkManagerTest extends UnitTestCase {
+    $language_manager = $this->createMock('Drupal\Core\Language\LanguageManagerInterface');
...
+    $this->moduleHandler = $this->createMock('\Drupal\Core\Extension\ModuleHandlerInterface');
...
+      $this->createMock('\Drupal\Core\Controller\ControllerResolverInterface'),

should we switch this to use the class constant whilst we're touching these lines?

Addressed #7.
@larowlan this is what you expected, Please have a look and advise.

According to me these lines are untouched as per the above patch, I think so there is no need to convert these(#11) lines.

I made changes suggested in #11 in the classes ViewAjaxControllerTest and ContentEntityBaseUnitTest.

Its not possible to alter PathPluginBaseTest because PathPluginBase is an abstract class, so we need to mock it. The same thing for EntityDisplayModeBaseUnitTest which mocks the abstract class EntityDisplayModeBase.

I bet we can do something like

class TestPathPlugin extends PathPluginBase {}

and then

$this->pathPlugin = new TestPathPlugin([], 'path_base', [], $this->routeProvider, $this->state);

to test the abstract classes?

EntityDisplayModeBaseUnitTest::testSet/GetTargetType() don't look like very good tests, they explicitly test the internals of the object via reflection.

I thought about #16 but don't you think it is better not to have one more class file and just mock the abstract class?
I think its easier to understand the test and we don't need a new file.

You don't need a new file, you can just declare it inside the same file as the test; we do this in a number of tests already. But I get your point, mocking is probably easier to read.

Ah I got it.
I still prefer using the mock because its easier to read.

I tested the patch and it applies normally on 9.4.x. Also, no sniff were found. I do think that mocking the abstract class is the easier to read option, and the patch at #15 addresses that. It also replaces all mocks (when possible and/or necessary) with its actual objects. I actually don't see any problems with the patch so, for me, is RTBC.

Updating version and re-uploading the patch from #14.

Less mocks for the win. Nice work. Backported to 9.3.x to keep tests aligned.

Committed and pushed de4c1e82c2 to 10.0.x and da098e3256 to 9.5.x and 95a1d4db5c to 9.4.x and e6247d2632 to 9.3.x. Thanks!