Problem

The following Unit Test does not pass in Windows environment due to hardcoded use of directory separator.

  • www/core/tests/Drupal/Tests/Component/FileCache/FileCacheTest.php
  • www/core/tests/Drupal/Tests/Component/ClassFinder/ClassFinderTest.php
  • www/core/tests/Drupal/Tests/Component/Discovery/YamlDirectoryDiscoveryTest.php

Proposed resolution

Use of DIRECTORY_SEPARATOR constant value instead of the hardcoding the directory separator in paths.
Attached in the patch file.

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

ProFire created an issue. See original summary.

ProFire’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, fix-for-unit-test-windows.patch, failed testing. View results

ProFire’s picture

ProFire’s picture

Status: Needs work » Needs review
michaellenahan’s picture

Issue tags: +Novice
michaellenahan’s picture

Issue tags: +Vienna2017
a.dmitriiev’s picture

The test results on Windows 10, PHP version 7.0.2

PS C:\xampp\htdocs\d8\core> php ..\vendor\phpunit\phpunit\phpunit .\tests\Drupal\Tests\Component\FileCache\FileCacheFactoryTest.php
PHPUnit 4.8.36 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\Component\FileCache\FileCacheFactoryTest
...........

Time: 8.69 seconds, Memory: 6.00MB

OK (11 tests, 13 assertions)

PS C:\xampp\htdocs\d8\core> php ..\vendor\phpunit\phpunit\phpunit .\tests\Drupal\Tests\Component\ClassFinder\ClassFinderTest.php
PHPUnit 4.8.36 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\Component\ClassFinder\ClassFinderTest
.

Time: 8.67 seconds, Memory: 6.00MB

OK (1 test, 4 assertions)

PS C:\xampp\htdocs\d8\core> php ..\vendor\phpunit\phpunit\phpunit .\tests\Drupal\Tests\Component\Discovery\YamlDirectoryDiscoveryTest.php
PHPUnit 4.8.36 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\Component\Discovery\YamlDirectoryDiscoveryTest
....

Time: 8.77 seconds, Memory: 6.00MB

OK (4 tests, 16 assertions)

a.dmitriiev’s picture

Status: Needs review » Reviewed & tested by the community

Works on Windows.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 4: 2911377-fix-for-unit-test.patch, failed testing. View results

a.dmitriiev’s picture

Status: Needs work » Reviewed & tested by the community

Just resave the issue, because the test failed on missing CI Job

xjm’s picture

Since this is a change that impacts the test runner, let's make sure we have a passing test run. I am re-queuing the test to make sure the missing job was unrelated.

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Needs review
  1. +++ b/core/tests/Drupal/Tests/Component/Discovery/YamlDirectoryDiscoveryTest.php
    @@ -80,18 +80,18 @@ public function testDiscovery() {
    -    $this->assertSame(['id' => 'item1', 'name' => 'test1 item 1', YamlDirectoryDiscovery::FILE_KEY => 'vfs://modules/test_1/subdir1/item_1.test.yml'], $data['test_1']['item1']);
    -    $this->assertSame(['id' => 'item2', 'name' => 'test1 item 2', YamlDirectoryDiscovery::FILE_KEY => 'vfs://modules/test_1/subdir2/item_2.test.yml'], $data['test_1']['item2']);
    +    $this->assertSame(['id' => 'item1', 'name' => 'test1 item 1', YamlDirectoryDiscovery::FILE_KEY => 'vfs://modules/test_1/subdir1' . DIRECTORY_SEPARATOR . 'item_1.test.yml'], $data['test_1']['item1']);
    +    $this->assertSame(['id' => 'item2', 'name' => 'test1 item 2', YamlDirectoryDiscovery::FILE_KEY => 'vfs://modules/test_1/subdir2' . DIRECTORY_SEPARATOR . 'item_2.test.yml'], $data['test_1']['item2']);
    ...
    -    $this->assertSame(['id' => 'item3', 'name' => 'test2 item 3', YamlDirectoryDiscovery::FILE_KEY => 'vfs://modules/test_2/subdir1/item_3.test.yml'], $data['test_2']['item3']);
    +    $this->assertSame(['id' => 'item3', 'name' => 'test2 item 3', YamlDirectoryDiscovery::FILE_KEY => 'vfs://modules/test_2/subdir1' . DIRECTORY_SEPARATOR . 'item_3.test.yml'], $data['test_2']['item3']);
    ...
    -    $this->assertSame(['id' => 'item4', 'name' => 'test4 item 4', YamlDirectoryDiscovery::FILE_KEY => 'vfs://modules/test_4/subdir1/item_4.test.yml'], $data['test_4']['item4']);
    -    $this->assertSame(['id' => 'item5', 'name' => 'test4 item 5', YamlDirectoryDiscovery::FILE_KEY => 'vfs://modules/test_4/subdir1/item_5.test.yml'], $data['test_4']['item5']);
    -    $this->assertSame(['id' => 'item6', 'name' => 'test4 item 6', YamlDirectoryDiscovery::FILE_KEY => 'vfs://modules/test_4/subdir1/item_6.test.yml'], $data['test_4']['item6']);
    +    $this->assertSame(['id' => 'item4', 'name' => 'test4 item 4', YamlDirectoryDiscovery::FILE_KEY => 'vfs://modules/test_4/subdir1' . DIRECTORY_SEPARATOR . 'item_4.test.yml'], $data['test_4']['item4']);
    +    $this->assertSame(['id' => 'item5', 'name' => 'test4 item 5', YamlDirectoryDiscovery::FILE_KEY => 'vfs://modules/test_4/subdir1' . DIRECTORY_SEPARATOR . 'item_5.test.yml'], $data['test_4']['item5']);
    +    $this->assertSame(['id' => 'item6', 'name' => 'test4 item 6', YamlDirectoryDiscovery::FILE_KEY => 'vfs://modules/test_4/subdir1' . DIRECTORY_SEPARATOR . 'item_6.test.yml'], $data['test_4']['item6']);
    
    @@ -113,7 +113,7 @@ public function testDiscoveryAlternateId() {
    -    $this->assertSame(['alt_id' => 'item1', 'id' => 'ignored', YamlDirectoryDiscovery::FILE_KEY => 'vfs://modules/test_1/item_1.test.yml'], $data['test_1']['item1']);
    +    $this->assertSame(['alt_id' => 'item1', 'id' => 'ignored', YamlDirectoryDiscovery::FILE_KEY => 'vfs://modules/test_1' . DIRECTORY_SEPARATOR . 'item_1.test.yml'], $data['test_1']['item1']);
    

    Why is it that only some slashes need to be converted/fixed in these?

  2. +++ b/core/tests/Drupal/Tests/Component/Discovery/YamlDirectoryDiscoveryTest.php
    @@ -124,7 +124,7 @@ public function testDiscoveryAlternateId() {
    -    $this->setExpectedException(DiscoveryException::class, 'The vfs://modules/test_1/item_1.test.yml contains no data in the identifier key \'id\'');
    +    $this->setExpectedException(DiscoveryException::class, 'The vfs://modules/test_1' . DIRECTORY_SEPARATOR . 'item_1.test.yml contains no data in the identifier key \'id\'');
    
    @@ -144,7 +144,7 @@ public function testDiscoveryNoIdException() {
    -    $this->setExpectedException(DiscoveryException::class, 'The vfs://modules/test_1/item_1.test.yml contains invalid YAML');
    +    $this->setExpectedException(DiscoveryException::class, 'The vfs://modules/test_1' . DIRECTORY_SEPARATOR . 'item_1.test.yml contains invalid YAML');
    

    Do we need to make the change in the test messages?

ProFire’s picture

1) Only some slashes are converted because from Line 60 to Line 78, the slashes are defined statically as forward slash. Thus the output of YamlDirectoryDiscovery($directories, 'test') at Line 80 will output exactly as statically defined.

Only the slashes that are not statically defined is using the DIRECTORY_SEPARATOR constant.

2) Yes. Similar to (1), on Line 135 and Line 155, the slashes are statically defined. The exception messages will look like these on Windows respectively:
The vfs://modules/test_1\item_1.test.yml contains no data in the identifier key 'id'
The vfs://modules/test_1\item_1.test.yml contains invalid YAML

Only the slashes that are not statically defined is using the DIRECTORY_SEPARATOR constant.

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

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should 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.