Comment entity acquired and releases the different locks

Is this still valid? test passes so whats wrong?

Re-roll after #1893772: Move entity-type specific storage logic into entity classes
I don't see a way to test this, because you need to detect that lock is acquired in Comment::preSave() before Comment::postSave() executed

Awesome! Test actually covers that

There was 1 failure:

1) Drupal\comment\Tests\Entity\CommentLockTest::testLocks
Expectation failed for method name is equal to <string:release> when invoked at sequence index 1.
The expected invocation at index 1 was never reached.


FAILURES!
Tests: 414, Assertions: 1201, Failures: 1.

+++ b/core/modules/comment/tests/Drupal/comment/Tests/Entity/CommentLockTest.php
@@ -0,0 +1,90 @@
+    $old_container = \Drupal::getContainer();
...
+    if ($old_container) {
+      \Drupal::setContainer($old_container);
+    }

Are these needed? I thought each phpunit test ran in a separate instance? If so can we get a comment as to why?

There is a --process-isolation flag for PHPUnit which defaults to FALSE, so unfortunately this is necessary.

+++ b/core/modules/comment/tests/Drupal/comment/Tests/Entity/CommentLockTest.php
@@ -0,0 +1,90 @@
+    if ($old_container) {
+      \Drupal::setContainer($old_container);
+    }
...
+    $old_container = \Drupal::getContainer();

We do need to reset the container, but we don't usually do it in the test method.

PHPUnit has both tearDown() and tearDownAfterClass(), and our container resets look like this:

  /**
   * {@inheritdoc}
   */
  protected function tearDown() {
    parent::tearDown();
    $container = new ContainerBuilder();
    \Drupal::setContainer($container);
  }

Looks great, thanks!

Committed/pushed to 8.x, thanks!