Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
ContainerBuilderTest is coupled to Symfony's tests, which means if they change their test, our test breaks. It also means we can't untangle the vendor/
-in-core/
problem here: #2380389: Use a single vendor directory in the root
Currently it requires a file in vendor/
, like this:
require_once __DIR__ . '../../../../../../vendor/symfony/dependency-injection/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/classes.php';
This is very brittle.
Proposed resolution
Re-implement the test classes (BarClass
, BazClass
) for the tests, using a \Drupal
namespace.
Remaining tasks
User interface changes
API changes
Beta phase evaluation
Issue category | Bug because our tests depend on something 2.7 no longer has! |
---|---|
Issue priority | Normal, even if its breaks at the moment. |
Unfrozen changes | Unfrozen because it only changes automated tests. |
Disruption | No disruption. |
Comment | File | Size | Author |
---|---|---|---|
#15 | 2445497_15.patch | 6.1 KB | Mile23 |
#15 | interdiff.txt | 2.78 KB | Mile23 |
#7 | interdiff.txt | 3.89 KB | Mile23 |
#7 | 2445497_6.patch | 5.66 KB | Mile23 |
#3 | interdiff.txt | 5.68 KB | Mile23 |
Comments
Comment #1
Mile23Adds a classes.php file right next to
ContainerBuilderTest
.ContainerBuilderTest
then requires that file instead.Comment #3
Mile23SimpleTest seems to want me to change this around. So I will. Probably for the better anyway.
Adding two classes under the
Fixture/
subnamespace.ContainerBuilderTest
also uses anotherBarClass
class from Symfony, but refers to it throughuse
rather thanrequire_once
, so that's livable.Comment #4
Mile23Comment #5
Mile23Marking as novice, because it's easy to review.
Comment #6
dawehnerMaybe a naive question, but if we want to decouple it from the symfony test, don't we want to not use their class as well then?
Comment #7
Mile23Not a naive question, really. :-)
I removed the AnnotatedBarClass entirely, because it wasn't really necessary.
Some coding standards improvements, too.
Comment #8
Mile23This patch still applies. Giving it another testbot go-round.
Comment #10
Mile23Comment #11
dawehnerSeems good for me
Comment #12
rteijeiro CreditAttribution: rteijeiro at Tieto commentedShould this be:
Contains \Drupal\Tests\Core\DependencyInjection\Fixture\BarClass.
?And this:
Contains \Drupal\Tests\Core\DependencyInjection\Fixture\BazClass.
?Maybe this property needs documentation?
Missing docblock here?
Same here?
The same for this property, should we document it?
And document this functions?
Comment #13
Mile23Comment #14
joelpittetDo you need the namespaced class in the register statements when you've added the use statements?
Comment #15
Mile23Yes. The container doesn't know what namespaces you've
use
d. It needs fully-quaified names.Fixed the coding standards issues from #12.
It turns out nothing uses BazClass' accessors or statics, so I killed them.
Thanks!
Comment #16
dawehnerLet's update the issue summary. This issue is needed in order to be able to update to symfony 2.7
Comment #17
Mile23Comment #18
alexpottCommitted 673f5b7 and pushed to 8.0.x. Thanks!
Thanks for adding the beta evaluation to the issue summary.