Skip to content
This repository has been archived by the owner on Sep 7, 2022. It is now read-only.

Crash without PHPUnit installed #2

Closed
jtojnar opened this issue May 31, 2022 · 17 comments
Closed

Crash without PHPUnit installed #2

jtojnar opened this issue May 31, 2022 · 17 comments

Comments

@jtojnar
Copy link

jtojnar commented May 31, 2022

Running vendor/bin/barista analyze app in a Nette project that does not use PHPUnit (nette/tester is okay) crashes:

Fatal error: Uncaught Error: Class "PHPUnit\Framework\TestCase" not found in /home/jtojnar/Projects/eudyptes/vendor/barista/barista/src/Testing/Macro/AbstractMacroTest.php:22
Stack trace:
#0 /home/jtojnar/Projects/eudyptes/vendor/composer/ClassLoader.php(571): include()
#1 /home/jtojnar/Projects/eudyptes/vendor/composer/ClassLoader.php(428): Composer\Autoload\includeFile('/home/jtojnar/P...')
#2 [internal function]: Composer\Autoload\ClassLoader->loadClass('Barista\\Testing...')
#3 /home/jtojnar/Projects/eudyptes/vendor/nette/di/src/DI/Extensions/SearchExtension.php(97): class_exists('Barista\\Testing...')
#4 /home/jtojnar/Projects/eudyptes/vendor/nette/di/src/DI/Extensions/SearchExtension.php(72): Nette\DI\Extensions\SearchExtension->findClasses(Object(stdClass))
#5 /home/jtojnar/Projects/eudyptes/vendor/nette/di/src/DI/Compiler.php(241): Nette\DI\Extensions\SearchExtension->loadConfiguration()
#6 /home/jtojnar/Projects/eudyptes/vendor/nette/di/src/DI/Compiler.php(211): Nette\DI\Compiler->processExtensions()
#7 /home/jtojnar/Projects/eudyptes/vendor/nette/di/src/DI/ContainerLoader.php(119): Nette\DI\Compiler->compile()
#8 /home/jtojnar/Projects/eudyptes/vendor/nette/di/src/DI/ContainerLoader.php(81): Nette\DI\ContainerLoader->generate('Container_55cc0...', Array)
#9 /home/jtojnar/Projects/eudyptes/vendor/nette/di/src/DI/ContainerLoader.php(44): Nette\DI\ContainerLoader->loadFile('Container_55cc0...', Array)
#10 /home/jtojnar/Projects/eudyptes/vendor/nette/bootstrap/src/Bootstrap/Configurator.php(274): Nette\DI\ContainerLoader->load(Array, Array)
#11 /home/jtojnar/Projects/eudyptes/vendor/nette/bootstrap/src/Bootstrap/Configurator.php(252): Nette\Bootstrap\Configurator->loadContainer()
#12 /home/jtojnar/Projects/eudyptes/vendor/barista/barista/src/DI/BaristaContainerFactory.php(32): Nette\Bootstrap\Configurator->createContainer()
#13 /home/jtojnar/Projects/eudyptes/vendor/barista/barista/bin/barista.php(31): Barista\DI\BaristaContainerFactory->create(Array)
#14 /home/jtojnar/Projects/eudyptes/vendor/barista/barista/bin/barista(4): require('/home/jtojnar/P...')
#15 /home/jtojnar/Projects/eudyptes/vendor/bin/barista(117): include('/home/jtojnar/P...')
#16 {main}
  thrown in /home/jtojnar/Projects/eudyptes/vendor/barista/barista/src/Testing/Macro/AbstractMacroTest.php on line 22
@jtojnar
Copy link
Author

jtojnar commented May 31, 2022

Even if I install phpunit, it still crashes with a different error:

Fatal error: Uncaught Nette\DI\ServiceCreationException: Service 'application.1' (type of App\Presenters\DashboardPresenter): Service of type App\Model\HelperLoader required by $helperLoader in App\Presenters\BasePresenter::__construct() not found. Did you add it to configuration file? in /home/jtojnar/Projects/eudyptes/vendor/nette/di/src/DI/Resolver.php:650
Stack trace:
#0 /home/jtojnar/Projects/eudyptes/vendor/nette/di/src/DI/Resolver.php(577): Nette\DI\Resolver::autowireArgument(Object(ReflectionParameter), Object(Closure))
#1 /home/jtojnar/Projects/eudyptes/vendor/nette/di/src/DI/Resolver.php(232): Nette\DI\Resolver::autowireArguments(Object(ReflectionMethod), Array, Object(Closure))
#2 /home/jtojnar/Projects/eudyptes/vendor/nette/di/src/DI/Definitions/ServiceDefinition.php(226): Nette\DI\Resolver->completeStatement(Object(Nette\DI\Definitions\Statement))
#3 /home/jtojnar/Projects/eudyptes/vendor/nette/di/src/DI/Resolver.php(168): Nette\DI\Definitions\ServiceDefinition->complete(Object(Nette\DI\Resolver))
#4 /home/jtojnar/Projects/eudyptes/vendor/nette/di/src/DI/ContainerBuilder.php(332): Nette\DI\Resolver->completeDefinition(Object(Nette\DI\Definitions\ServiceDefinition))
#5 /home/jtojnar/Projects/eudyptes/vendor/nette/di/src/DI/Compiler.php(275): Nette\DI\ContainerBuilder->complete()
#6 /home/jtojnar/Projects/eudyptes/vendor/nette/di/src/DI/Compiler.php(212): Nette\DI\Compiler->processBeforeCompile()
#7 /home/jtojnar/Projects/eudyptes/vendor/nette/di/src/DI/ContainerLoader.php(119): Nette\DI\Compiler->compile()
#8 /home/jtojnar/Projects/eudyptes/vendor/nette/di/src/DI/ContainerLoader.php(81): Nette\DI\ContainerLoader->generate('Container_55cc0...', Array)
#9 /home/jtojnar/Projects/eudyptes/vendor/nette/di/src/DI/ContainerLoader.php(44): Nette\DI\ContainerLoader->loadFile('Container_55cc0...', Array)
#10 /home/jtojnar/Projects/eudyptes/vendor/nette/bootstrap/src/Bootstrap/Configurator.php(274): Nette\DI\ContainerLoader->load(Array, Array)
#11 /home/jtojnar/Projects/eudyptes/vendor/nette/bootstrap/src/Bootstrap/Configurator.php(252): Nette\Bootstrap\Configurator->loadContainer()
#12 /home/jtojnar/Projects/eudyptes/vendor/barista/barista/src/DI/BaristaContainerFactory.php(32): Nette\Bootstrap\Configurator->createContainer()
#13 /home/jtojnar/Projects/eudyptes/vendor/barista/barista/bin/barista.php(31): Barista\DI\BaristaContainerFactory->create(Array)
#14 /home/jtojnar/Projects/eudyptes/vendor/barista/barista/bin/barista(4): require('/home/jtojnar/P...')
#15 /home/jtojnar/Projects/eudyptes/vendor/bin/barista(117): include('/home/jtojnar/P...')
#16 {main}
  thrown in /home/jtojnar/Projects/eudyptes/vendor/nette/di/src/DI/Resolver.php on line 650

Perhaps it is not supposed to be installed with composer require barista/barista --dev?

@jtojnar
Copy link
Author

jtojnar commented May 31, 2022

Running composer install in vendor/barista/barista/ seems to fix it but is that really expected?

@TomasVotruba
Copy link
Member

The 1st error - for some reason it tries to register this testcase as a service:
https://github.com/TomasVotruba/barista/blob/main/src/Testing/Macro/AbstractMacroTest.php

Would excluding the class in here help?
https://github.com/TomasVotruba/barista/blob/b1ee06bfeaa621605ba0adfc901b94fcdfe68654/config/services.neon#L5

@jtojnar
Copy link
Author

jtojnar commented May 31, 2022

Excluding does not seem to work.

The search extension checks for the class existence

https://github.com/nette/di/blob/1b2d3716c51f36f1b71780d5b0594cd0a258fa99/src/DI/Extensions/SearchExtension.php#L93

before checking if it is excluded

https://github.com/nette/di/blob/1b2d3716c51f36f1b71780d5b0594cd0a258fa99/src/DI/Extensions/SearchExtension.php#L108

I verified by dumping $config here and it only ever prints:

object(stdClass)#205 (7) {
  ["in"]=>
  string(71) "/home/jtojnar/Projects/eudyptes/vendor/barista/barista/src/DI/../../src"
  ["exclude"]=>
  object(stdClass)#201 (3) {
    ["classes"]=>
    array(3) {
      [0]=>
      string(39) "Barista\Testing\Macro\AbstractMacroTest"
      [1]=>
      string(34) "Barista\DI\BaristaContainerFactory"
      [2]=>
      string(28) "Barista\Configuration\Option"
    }
    ["implements"]=>
    array(1) {
      [0]=>
      string(39) "Barista\Contract\LatteUpgraderInterface"
    }
    ["extends"]=>
    array(0) {
    }
  }
  ["files"]=>
  array(0) {
  }
  ["classes"]=>
  array(0) {
  }
  ["extends"]=>
  array(0) {
  }
  ["implements"]=>
  array(0) {
  }
  ["tags"]=>
  array(0) {
  }
}

@TomasVotruba
Copy link
Member

I see. And the class existance probably invokes the parent class autoloading.
What do you suggest to overcome this?

@jtojnar
Copy link
Author

jtojnar commented May 31, 2022

You are probably more qualified to answer that.

Is it not possible to use Composer autoloading for Nette autowiring? It seems to work pretty well in app projects (unless Nette somehow still uses RobotLoader internally) but I am not really sure how it is supposed to work in tools installed as Composer dependencies. I would expect them to just use the autoloader of the parent project but removing the search section of the config just results in broken autowiring:

Fatal error: Uncaught Nette\DI\MissingServiceException: Service of type Barista\FileSystem\LatteFilesFinder not found. Did you add it to configuration file? in /home/jtojnar/Projects/eudyptes/vendor/nette/di/src/DI/Autowiring.php:57

@TomasVotruba
Copy link
Member

TomasVotruba commented May 31, 2022

I don't know how exactly the search componet works.

I think that to make code work, we have to:

  • move the test case class
  • or add phpunit as required dependency

It should be still available without installing phpunit as extra dependency.

@jtojnar
Copy link
Author

jtojnar commented May 31, 2022

It loads the app’s autoload file and vendor/composer/autoload_psr4.php contains 'Barista\\' => array($vendorDir . '/barista/barista/src'),.

Actually looking at the autowiring code, it is able to find the class, or it would crash with a different error. Will do more dumping.

@TomasVotruba
Copy link
Member

My rought guess is that it crashes on class_parents() here:
https://github.com/nette/di/blob/9878f2958a0a804b08430dbc719a52e493022739/src/DI/Autowiring.php#L106

@jtojnar
Copy link
Author

jtojnar commented May 31, 2022

That behaviour is probably intentional so that Nette DI does not create random classes.

Another alternative could be removing the RobotLoader in favour of explicitly listed factories. The following seems to work for the analyze command:

--- a/config/services.neon
+++ b/config/services.neon
@@ -1,14 +1,3 @@
-# autodiscovery, see https://doc.nette.org/en/dependency-injection/configuration#toc-search
-search:
-    in: %appDir%/../../src
-    exclude:
-        classes:
-            - Barista\DI\BaristaContainerFactory
-            - Barista\Configuration\Option
-        implements:
-            # register manually the rules you want to apply
-            - Barista\Contract\LatteUpgraderInterface
-
 decorator:
     Barista\Command\AbstractBaristaCommand:
         inject: true
@@ -21,6 +10,9 @@
         class: Barista\Command\LintCommand
         arguments:
             latteProviderFile: %latteProviderFile%
+
+    - Barista\FileSystem\LatteFilesFinder
+    - Barista\Console\ApplicationFactory
 
     - Latte\Compiler\TemplateParser
     - Latte\Compiler\TemplateLexer

Well, “work” it will then print the same error as in the second comment. But that is probably unrelated issue.

@TomasVotruba
Copy link
Member

That would force us to register everything manually and kill the point of Nette 3.0 search feature.

@jtojnar
Copy link
Author

jtojnar commented May 31, 2022

My rought guess is that it crashes on class_parents() here:
nette/di@9878f29/src/DI/Autowiring.php#L106

No, it crashed in Composer autoloader code triggered by the class_exist call in the search extension on the AbstractMacroTest. That was pretty clear from the initial trace.

That would force us to register everything manually and kill the point of Nette 3.0 search feature.

Not everything, just the stuff we want auto-wired. Which, at the moment seems to be the two classes listed in my patch.

To be honest, I have always felt that RobotLoader is a misfeature since Composer already has all the necessary information available statically (as long as you use Composer autoloading – but there is no excuse to avoid it these days, IMO). Unless I am missing something, the search extension should be able to look at all the explicitly registered services and try to register their dependencies, rather than scanning a directory and registering all classes it finds it does now.

@TomasVotruba
Copy link
Member

I want to keep the search here, to allow scaling services without thinking about manual registration. I'm doing the same in Symfony last 4 years.

@TomasVotruba
Copy link
Member

This should fix it: 08fa2df

Barista is a dev tool with abstract test case, so the PHPUnit is expected here.
Could you create a new issue for the application case? Ideally with small reproducer repository, as it seems very random.

@jtojnar
Copy link
Author

jtojnar commented May 31, 2022

Thanks, will do. I have opened nette/di#282 just in case it ditching the RobotLoader is interesting for David.

@jtojnar
Copy link
Author

jtojnar commented May 31, 2022

Though pondering it a bit more, the abstract test case is not really relevant to the dev tool so it might be cleanest to split it into a separate composer package. Though I understand that adding an extra dependency is probably the most pragmatic choice.

@TomasVotruba
Copy link
Member

At first sight, it would hell to maintain for a just a few classes.

Yet the test case is very useful for the upgrade. It should be part of the Latte 2 project already, while this package runs on Latte 3.
I might re-thing to drop it and separate in the future. This package is really young so we'll see where it grows :). I didn't expect any activity first months TBH.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

2 participants