Problem/Motivation

A number of Drupal modules and subsystems work with discovery in the file system.

These include but are not limited to modules/extensions and class autoloading.

These systems need tests, especially unit tests, since they are so central to the operation of Drupal, and also subject to being changed and refactored.

Proposed resolution

So let's add vfsStream as a dependency in composer.json.

What is vfsStream? It lives here: http://vfs.bovigo.org/ with a repo here: https://github.com/mikey179/vfsStream

From the site: vfsStream is a PHP stream wrapper for a virtual file system that may be helpful in unit tests to mock the real file system. It can be used with any unit test framework, like PHPUnit or SimpleTest.

With vfsStream, you virtualize the file system into a memory-based structure. This allows you to perform all kinds of file system operations under test, without touching the real file system.

As an example, you can have code like this: $this->assertTrue(is_dir(vfsStream::url('root/core/lib/Drupal/Core')));.

The mocked file system can be created easily from a code-based array under version control. It includes provisions for changing permissions and testing against them.

The mocked file system can also be read in from an existing file system.

Remaining tasks

Add vfsStream to composer.json.

Create subclass of \Drupal\Tests\UnitTestCase with some vfsStream utility methods, including a master mock Drupal file system array.

Convert existing tests which have dependencies on the file system to use the virtualized file system.

Refactor some code to have the file system injected as a dependency, rather than hard-coding DRUPAL_ROOT or __DIR__.

User interface changes

None.

API changes

None.

#2083547: PSR-4: Putting it all together
#2024083: [META] Improve the extension system (modules, profiles, themes)

CommentFileSizeAuthor
#9 2095037.png11.64 KBclemens.tolboom
#1 2095037_do-not-test.patch4.16 KBMile23
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Mile23’s picture

FileSize
4.16 KB

Here's a patch illustrating some of the concepts.

It would fail or be incomplete in test, so shoo, testbot!

To make it work: Apply the patch, update with Composer, and run ./vendor/bin/phpunit.

jibran’s picture

Refactor some code to have the file system injected as a dependency, rather than hard-coding DRUPAL_ROOT or __DIR__.

As much as I hate doing this and it seems nice addition but tbh it is D9 material even if we want to discuss it.

Mile23’s picture

I suffer no illusions. :-) But wouldn't it be useful?

dawehner’s picture

Category: Feature request » Task
Priority: Normal » Major
Issue tags: +DX (Developer Experience)

This is totally worth to add, as it does not interact with any actual code and allows people to write better/actual unit tests.

donquixote’s picture

Yes, this would be awesome for unit tests.
It means that stuff can move from fixtures or example modules into the test class itself. Which is great, because then you don't need to look around in various folders to understand what a test does.

A consequence is that such test classes become bigger. But we can simply write more and smaller test classes.

clemens.tolboom’s picture

+++ b/core/tests/Drupal/Tests/MockDrupalFileSystemTestCaseTest.php
@@ -0,0 +1,66 @@
+ *
+ * A test case being a subclass of the class under test seems weird. And yet
+ * is appropriate.

This is no documentation to me. Please explain why is it appropriate? :-)

As 'I don't understand' I may be wrong but extending phpunit with a filesystem thingy feels weird. My unit test needs a file system to work with fixtures right?

I somehow expected a StreamWrapper construct too.

(my 2cnts)

Mile23’s picture

@clemens.tolboom:

vfsStream allows you to substitute an in-memory filesystem for an on-disk one. You make a big array that looks like a file structure and then test the behavior of file operations on it. It allows you to get a lot more coverage and specificity for tests that would be impractical if you need a big mock filesystem buried somewhere to accomplish.

Furthermore, when it goes out of scope, it VANISHES. :-)

tim.plunkett’s picture

A test case being a subclass of the class under test seems weird. And yet is appropriate.

This is like a code review comment, not inline documentation.

clemens.tolboom’s picture

FileSize
11.64 KB

@all I was wrong about the inheritance chain.

@Mile23 I understand the construct of vfsStream but not it's usage in a Drupal world

My question was about streamwrapper-ness of vfsStream

// a 'normal' streamwrapper
$this->assertTrue(is_dir('public:///root/modules/fooModule'));

// vfs as a 'normal' streamwrapper
$this->assertTrue(is_dir('vfs:///root/modules/fooModule'));

// vfs 'awkward' construct to test against
$this->assertTrue(is_dir(vfsStream::url('root/modules/fooModule')));

It would be great for vfs to _be_ a streamwrapper I guess in lowering the learning curve.

@tim.plunkett even if it's a review comment it does not help me :-/

Anyway I guess this is a great addition as already mentioned by @Mile23 it moves test file artifacts into code.

(my credits-- cnts)

dawehner’s picture

Status: Active » Closed (duplicate)
sun’s picture

Component: other » phpunit

It helps to categorize issues correctly...

Mile23’s picture

You mean with 'vfsstream' in the title? :-)

Anyway, not 'sadly' at all. Doo eet.

sun’s picture

I meant the issue category/component, sorry.

Issues that target Simpletest or PHPUnit directly should be categorized under the corresponding components. Me and other testing system maintainers are actively tracking those two queues. If issues are wrongly categorized, they'll either get unnoticed or duplicated.

Due to this incident, I already tried to double-check the 'phpunit' tag queue for similar cases, but unfortunately the list is exceptionally large... — Why on earth are we tagging all issues that just happen to add or touch phpunit tests with "phpunit"...? Sad panda. :-(