Vocabulary restriction not working in Entity reference fields

Looks good to me. Confirming the bug and the fix.

Might be an idea to upload just the tests to ascertain that they fail without the fix.

Upping to major in light of #1847596: Remove Taxonomy term reference field in favor of Entity reference.

@joachim: Thanks for finding this.

Fix is mostly the same as mine over in #2247953: Entity reference select widget ignores bundle filter that you closed as duplicate. I like this, makes it a bit more readable than my quickfix.

Test looks good, just a bunch of nitpicks on coding standards and documentation. Tagging novice to fix that, then it should be ready.

1. +++ b/core/modules/taxonomy/src/Tests/TermSelectionTest.php
   @@ -0,0 +1,80 @@
   + * @file
   + * Definition of Drupal\taxonomy\Tests\TermSelectionTest.
   

   Should be Contains \Drupal... (we should really fill a task to get rid of all the old ones, people always copy wrong examples).

2. +++ b/core/modules/taxonomy/src/Tests/TermSelectionTest.php
   @@ -0,0 +1,80 @@
   +class TermSelectionTest extends TaxonomyTestBase {
   

   TermEntityReferenceTest maybe?

3. +++ b/core/modules/taxonomy/src/Tests/TermSelectionTest.php
   @@ -0,0 +1,80 @@
   +
   +  public static $modules = array('entity_reference', 'entity_reference_test', 'entity_test');
   

   Should have the usual "Modules to enable.\n@var array" docblock.

4. +++ b/core/modules/taxonomy/src/Tests/TermSelectionTest.php
   @@ -0,0 +1,80 @@
   +
   +  protected function setUp() {
   +    parent::setUp();
   +  }
   

   No need to keep an empty setUp() method.

5. +++ b/core/modules/taxonomy/src/Tests/TermSelectionTest.php
   @@ -0,0 +1,80 @@
   +  /**
   +   * Create two vocabularies with a term, then set up the entity reference field
   +   * to limit the target vocabulary to one of them, ensuring that that
   +   * the restriction applies.
   +   */
   +  function testSelectionTestVocabularyRestriction() {
   +
   

   The first line needs to be a single line (stupid for tests, but that's the rule atm..). Just make a sentence of the test method name... You can keep the existing part as a description below that.

6. +++ b/core/modules/taxonomy/src/Tests/TermSelectionTest.php
   @@ -0,0 +1,80 @@
   +    $field_storage = entity_create('field_storage_config', array(
   ...
   +    $field = entity_create('field_config', array(
   

   Should use FieldStorageConfig::create()/FieldConfig::create()

7. +++ b/core/modules/taxonomy/src/Tests/TermSelectionTest.php
   @@ -0,0 +1,80 @@
   +          // Restrict selection of terms to a single vocabulary
   

   Comment is missing a . at the end.

I'll work on the review by Berdir

+++ b/core/modules/taxonomy/src/Tests/TermEntityReferenceTest.php
@@ -15,17 +17,34 @@
+  /**
+   * A field storage to use in this test class.
+   *
+   * @var \Drupal\field\Entity\FieldStorageConfig
+   */
+  protected $field_storage;
+

I don't think that's necessary, that would only be useful if we would use them in multiple methods.

Everything else looks fine.

Pro-tip: Upload the fail patch first, the testbot only sets the issue status to needs work if the last uploaded patch fails.

+++ b/core/modules/taxonomy/src/Tests/TermEntityReferenceTest.php
@@ -0,0 +1,85 @@
+use Drupal\field\Entity\FieldStorageConfig;
+use Drupal\field\Entity\FieldConfig;
+use Drupal\taxonomy\Entity\Term;
+use Drupal\taxonomy\Plugin\entity_reference\selection\TermSelection;

One last nitpick, sorry.

I don't think the Term and TermSelection classes are used in the test?

You are correct :)

+++ b/core/modules/taxonomy/src/Tests/TermEntityReferenceTest.php
@@ -2,11 +2,13 @@
- * Definition of Drupal\taxonomy\Tests\TermSelectionTest.
+ * Contains \Drupal\taxonomy\Tests\TermEntityReferenceTest.
  */

I believe Berdir was referring to this change. There are a lot of test files where the TestClass docblock looks like this currently in HEAD.

Great, I think this is good to go :)

Yes, I just meant the @file declaration. But even of those, there are almost 1000 in core. But every second patch has that wrong because people copy from other files. That's what we get for changing a standard three times ;)

This issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed 7bb4e4d and pushed to 8.0.x. Thanks!