Disable profiler routes

[tacman]

I'm trying to disable the profiler when generating the functional tests. According to https://symfony.com/doc/current/profiler.html, the profiler service should be available via autowiring, but I keep getting an error.

#[AsCommand('generate:smoke', 'Generate the smoke test')]
final class MakeSmokeTestCommand extends Command
{

    public function __construct(
        private readonly RouterInterface                          $router,
        #[Autowire('%kernel.project_dir%/tests/')] private string $testDir,
        ?Profiler $profiler=null,
    )
    {
        parent::__construct();
        $profiler?->disable();
    }

In DefinitionErrorExceptionPass.php line 48:

[Symfony\Component\DependencyInjection\Exception\RuntimeException]
Cannot autowire service "Survos\CrawlerBundle\Command\CrawlCommand": argument "$profiler" of method "__construct()" references class "Symfony\Component\HttpKernel\Profiler\Profiler" but no such service exists. You should maybe alias this class to the existing "profiler" service.

Any ideas?

I know we can filter the routes other ways, but it seems like disabling the profiler altogether would be better.

[Pierstoval]

What about using the #[Autowire('profiler')] in the $profiler constructor argument ?

[tacman]

yeah, that didn't work either. I injected into the services and that finally worked:

# services.yaml
    Symfony\Component\HttpKernel\Profiler\Profiler: '@profiler'

But that didn't disable the routes :-( So I tweaked the generator, but it's not very elegant.

        foreach ($routes as $route) {
            // these patterns could also be defined in the bundle
            foreach (['_wdt_','_profiler_'] as $prefix) {
                if (str_starts_with($route['routeName'], $prefix)) {
                    continue 2;
                }
            }

[Pierstoval]

Yes, I guess filtering like that is maybe the only proper solution anyway

[tacman]

What do you think of an 'excluded routes' and 'excluded paths' properties in the bundle?

We would need to add a configuration extension, which is much easier to do if we extend AbstractBundle (Symfony 6+), and then the configuration can be embedded.

Then developers could configure routes and patterns that should be ignored, e.g. test-sentry (triggers an error to make sure sentry messages are being sent), etc.

[Pierstoval]

I'm not sure. Generating the routes is a tedious task in general, and generating code isn't the way I usually go. I prefer abstractions like the RouteExtractor for instance, but I don't really see a way to customize it apart with environment variables, or, like with the SmokeTestStaticRoutes class, by customizing runtime calls rather than the extractor itself.

[Pierstoval]

To add more details to my argument: generated code shouldn't be 100% perfect and work as-is, it usually never does, and has to be reviewed anyway. If you use the maker to generate tests for all your routes at the beginning of the project, it's the route extractor you'll want to customize to avoid testing some routes, but since everything will be in the tests anyway, that'll be a whole lot of steps to generate code, review, code other features, re-generate new code, re-review, etc.

I really think it's not at all what I would like to recommend to anyone.

Instead, the SmokeTestStaticRoutes does precisely this work, while still being quite easy to customize to filter the routes, and auto-updating since you don't have to regenerate your tests to create new smoke tests.

Remember this is a smoke-testing tool: it's just here to make sure to have a small layer of tests that can tell when parts of the project breaks. The code generation is here mostly to generate a bigger test suite on legacy projects that have zero tests, for instance, and review them all in order to make sure all of them can be green quickly. It's not meant for "new projects", which will see new routes very often, and code generation is useless here, or needs too much post-generation work. Exhaustive test suites will certainly be produced later on, but these will go out of the scope of a Smoke Testing tool, and will probably have to be written by hand anyway, code generation won't help there.

I sincerely appreciate all the improvements you're giving to the project, but the Maker is probably not the one that should take all the focus IMO: I personally only used it once, and on all the projects I used this library, I mostly customize the SmokeTestStaticRoutes class with a few tricks here and there, mostly to filter routes and add authentication on certain parts like administration panel.

Something I'd like to do, but it's way harder as it's about digging into PHPUnit logging, is to add some kind of "non-blocking warning" when a route returns a 4xx code, because tests will still be green in such case, but on an admin panel, you can have dozens, if not hundreds, of routes, and they'll all return 401 or 403, and I'd like to find a graceful way to warn users that these routes need authentication, so that they could just implement the beforeRequest() method. Maybe I'll work on it, I don't know, my time is very limited anyway.

[tacman]

Thanks for your detailed answer.

I like seeing the generated routes, it takes some of the mystery out of what's being tested. But I understand that continually regenerating the code has its own set of problems.

I have a bundle that does route testing by first crawling the site as different users. In predominately read-only sites, I've been able to get impressive code coverage with just these two bundles. I've recently refactored it to insert the TestWith attribute (rather than reading from a JSON file that was created during the crawl). But I may not keep that approach for the reasons you've mentioned.

[tacman]

add authentication on certain parts like administration panel.

Can you show an example? #19