Problem/Motivation

\Drupal\Tests\Component\Serialization\YamlTest::providerYamlFilesInCore() scans all yaml files in ./core. Once we have a js build process we'll have core/node_modules and this can have yml inside. We shouldn't be scanning in there.

Proposed resolution

Stop scanning in core/node_modules.

Steps to reproduce:

  1. Applied the latest on #2488860: Bring phpunit bridge into drupal and use it for unit tests and simpletest to handle Deprecation - this detects deprecated core path and Symfony 3 contains a deprecated code path for duplicate yaml keys.
  2. Go to ./core and run npm install
  3. run phpunit tests/Drupal/Tests/Component/Serialization/YamlTest.php

Note you will need the pecl yaml parser installed.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#11 2874904-drupal8.4-10.patch1.04 KBMixologic
#11 2874904-drupal8.5-10.patch1.04 KBMixologic
#7 2874904-7.patch1.04 KBMixologic
#7 2874904-7.patch69 bytesMixologic
#3 2874904-3.patch1.03 KBalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Issue summary: View changes
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Active » Needs review
FileSize
2874904-3.patch1.03 KB

So the vendor exclude is wrong because we're scanning from core/.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner at Chapter Three commented
Status: Needs review » Reviewed & tested by the community

Thank you @alexpott
This fix is needed for not failing when yarn installis executed on the testbot.

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
CreditAttribution: Mixologic at Drupal Association commented
Status: Reviewed & tested by the community » Needs review
FileSize
2874904-7.patch69 bytes
2874904-7.patch1.04 KB

Needed a reroll

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
CreditAttribution: Mixologic at Drupal Association commented
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner at Chapter Three commented
Status: Needs review » Reviewed & tested by the community 
+++ b/core/tests/Drupal/Tests/Component/Serialization/YamlTest.php
@@ -84,8 +84,8 @@ public function providerYamlFilesInCore() {
-      // Exclude vendor.
-      if ($dir->getExtension() == 'yml' && strpos($pathname, '/../../../../../vendor') === FALSE) {
+      // Exclude core/node_modules.
+      if ($dir->getExtension() == 'yml' && strpos($pathname, '/../../../../../node_modules') === FALSE) {

I was about to suggest: Using $this->core/node_modules would be way more readable, but then I realized, this test is for a component, too bad.

The last submitted patch, 7: 2874904-7.patch, failed testing. View results

Mixologic’s picture

Mixologic
He/Him/His
Primary language English
Location Portland, OR
CreditAttribution: Mixologic at Drupal Association commented
FileSize
2874904-drupal8.5-10.patch1.04 KB
2874904-drupal8.4-10.patch1.04 KB

Turns out that 8.4.x, 8.5.x and 8.6.x have different contents in that file.

  • larowlan committed 409b393 on 8.6.x 
    Issue #2874904 by Mixologic, alexpott: Fix YamlTest to not scan core/...

  • larowlan committed b6408bb on 8.5.x 
    Issue #2874904 by Mixologic, alexpott: Fix YamlTest to not scan core/...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Version: 8.6.x-dev » 8.4.x-dev
Status: Reviewed & tested by the community » Fixed

Committed as 409b393 and pushed to 8.6.x

Cherry-picked as b6408bb and pushed to 8.5.x

Even though 8.4.x is criticals only, this impacts the test infrastructure, and we're going to want to keep being able to test on 8.4, so committed 625a4d6 and pushed to 8.4.x.

Thanks folks

  • larowlan committed 625a4d6 on 8.4.x 
    Issue #2874904 by Mixologic, alexpott: Fix YamlTest to not scan core/...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.