phpunit/php-file-iterator (a dependency of PHPUnit) 1.3.4 adds the possibility to follow symlinks when searching for test cases. Many Drupal developers symlink their contributed modules into a single Drupal install they use as a testbed, and upgrading to 1.3.4 will allow them to run all PHPUnit tests in that installation at once.

Files: 
CommentFileSizeAuthor
#11 drupal_2110153_11.patch28.01 KBXano
PASSED: [[SimpleTest]]: [MySQL] 59,366 pass(es).
[ View ]
#4 php-file-iterator_2110153_4.patch18.05 KBMile23
PASSED: [[SimpleTest]]: [MySQL] 58,584 pass(es).
[ View ]
#4 php-file-iterator_composer-only_2110153_4_do-not-test.patch381 bytesMile23
#1 drupal_2110153_1.patch17.45 KBXano
PASSED: [[SimpleTest]]: [MySQL] 58,938 pass(es).
[ View ]

Comments

Assigned:Xano» Unassigned
Status:Active» Needs review
StatusFileSize
new17.45 KB
PASSED: [[SimpleTest]]: [MySQL] 58,938 pass(es).
[ View ]

I'll agree that this is vital. symlinks are quite common and making them invisible when running tests is a bit surprise.

+++ b/composer.lock
@@ -774,16 +774,16 @@
-                "url": "git://github.com/sebastianbergmann/php-file-iterator.git",
-                "reference": "1.3.3"
+                "url": "https://github.com/sebastianbergmann/php-file-iterator.git",
+                "reference": "acd690379117b042d1c8af1fafd61bde001bf6bb"

Hm. Why are we referencing a specific commit hash here and not a version?

StatusFileSize
new381 bytes
new18.05 KB
PASSED: [[SimpleTest]]: [MySQL] 58,584 pass(es).
[ View ]

Changed the php-file-iterator version in composer.json, did composer update phpunit/php-file-iterator, and here's the patch.

A maintainer might only add the composer.json change and then do a full update themselves.

Also, you can see the latest available versions of stuff on packagist: https://packagist.org/packages/phpunit/php-file-iterator

That patch has the same hash problem. I'm also not sure adding the dependency to core is a good idea, as core itself does not directly depend on it.

The .lock file is supposed to be generated by composer. It'll be obliterated the next time you run composer update. This version of php-file-iterator is, actually, a dependency of Drupal, so therefore we write that down in composer.json.

#1: drupal_2110153_1.patch queued for re-testing.

The .lock file is supposed to be generated by composer. It'll be obliterated the next time you run composer update.

Can you explain what you mean with this?

This version of php-file-iterator is, actually, a dependency of Drupal, so therefore we write that down in composer.json.

It is not. Core does not use php-file-iterator. Only PHPUnit does. We just want to pull in the latest version, because it will benefit contrib. It's technically a feature to make development easier.

The .lock file is generated by composer. It's purpose in life is to store the dependency information generated when you type composer update. If it's present, typing composer install will cause composer to read the lock file and not have to work out the dependencies, which can be time-consuming. It gets over-written when you do another update. http://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file

Whether the dependency is satisfied by a git hash or a file download or whatever is determined by the package. We don't care about that, which is why we don't care about the .lock file, other than it being there.

If the project requires a certain version of a package, it should go in the composer.json file. That way it's explicit that for things to work, you need this version of the package. We want contrib to be able to use PHPUnit, so therefore we need this version of php-file-iterator or later.

If Drupal decides it doesn't want to use PHPUnit any more, then it's a matter of deleting two lines in composer.json instead of one. Ideally, all this would be documented in the composer.json file, but json doesn't like comments.

Also: php-file-iterator is pretty cool. It's not like Drupal ever needs to recursively find files based on suffixes. :-) We could be using php-file-iterator, since it's already there.

Status:Needs review» Needs work

This looks fine everywhere, but this does not apply anymore as you basically have to rerole composer changes everytime any dependency is updated.

Status:Needs work» Needs review
StatusFileSize
new28.01 KB
PASSED: [[SimpleTest]]: [MySQL] 59,366 pass(es).
[ View ]

Category:feature» bug
Priority:Major» Critical

In light of the upcoming alpha 4 release the code sprints at BADCamp, in Edinburgh, and in Manchester next week, I'm temporarily bumping this to critical, so we can hopefully get this in alpha 4 and contrib developers can use PHPUnit when porting their code.

Also, seeing as this is a regression compared to Simpletest, I'm making this a bug report, rather than a feature request.

Have a quick look at #2105821-8: Updated Symfony CMF's Routing Component to v1.1.0

Your patch basically reverts all the changes done in that composer files, so I am wondering whether you have a recent composer version.

Your description sounds really a lot like major, to be honest:

Major: For issues which have significant repercussions, but do not render the whole system unusable.

but yeah actually I just wanted to look up the definition of critical/major, so I don't change it.

I have the very latest Composer and all I did to create the patch was composer update phpunit/php-file-iterator.

I had a short conversation with @seldaek in #composer-dev on IRC. He confirmed that the changes in PHP logic are as planned:

[3:26pm] <-Xano>
Hi folks! I'm here on behalf of Drupal core. We use Composer to manage some dependencies, but during the latest update we noticed quite a few changes in the PHP code of some of the files
[3:26pm] <-Xano>
We just wanted to verify whether that is correct or not
[3:26pm] <-Xano>
https://drupal.org/files/drupal_2110153_11.patch is our diff
[3:26pm] <-Xano>
See ClassLoader.php for good examples
[3:33pm] <-Seldaek>
Xano: this all seems legit
[3:34pm] <-Seldaek>
we did some improvements to the ClassLoader code a while back
[3:34pm] <-Seldaek>
the rest is just regular update of the autoloader because you have new/diff packages

Status:Needs review» Reviewed & tested by the community

Thank you, so the changes made by cosmicdreams was actually wrong.

Status:Reviewed & tested by the community» Fixed

I reran the process of updating to the new php-file-iterator and can confirm that the patch only contains expected changes.

Committed 6a89adc and pushed to 8.x. Thanks!

Status:Fixed» Closed (fixed)

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