Feature tests break with module enabled

[dstepe]

We've run into two problems when running feature tests in Laravel with the module enabled. First, the agent attempts to send the transaction to APM regardless of the APM_ACTIVE value. I believe this is due to the agent performing a type check on config value ($this->config->get('active') === false). When setting the env value via phpunit.xml, the string false is set as an empty string in configuration. This could be addressed in the elastic-apm.php config with something like this:

'active'        => env('APM_ACTIVE', true) === true,

With this change, configuration would always have a properly typed value based on a clear positive assertion from the user. The default would not change, so I think this is safe.

I think it also makes sense to add documentation encouraging the creation of an .env.testing file with APM_ACTIVE=false explicitly set.

The second problem is the same as discussed in issue https://github.com/philkra/elastic-apm-laravel/issues/16 and is related to the LARAVEL_START constant. The conclusion in the discussion is that there is some way of running PHPUnit tests through Laravel's bootstrap process, citing:

https://laravel.com/docs/5.7/testing#creating-and-running-tests

That documentation simply says "To run your tests, execute the phpunit command from your terminal" and does not provide a Laravel bootstrapped method of running tests. The artisan command can be used to create a test, but does not actually run them from what I'm reading.

I'd appreciate further clarification on running the feature and possibly revisiting this as an issue which needs to be addressed. We have many applications which use feature tests and our build process runs tests with PHPunit directly, so we can't make widespread use of the module until we have a solution.

[dstepe]

I was digging into the LARAVEL_START issue and found some mention of this issue in the Laravel DebugBar project. That was in older versions and LARAVEL_START isn't mentioned in the current code. I finally tracked down how the start time is derived and think this module would benefit from the same approach.

Take a look at:

https://github.com/barryvdh/laravel-debugbar/blob/master/src/LaravelDebugbar.php#L151

The start time is derived from:

$startTime = $this->app['request']->server('REQUEST_TIME_FLOAT');

The REQUEST_TIME_FLOAT is provided by PHP:

"The timestamp of the start of the request, with microsecond precision. Available since PHP 5.4.0."

http://php.net/manual/en/reserved.variables.server.php

I will work on a PR for this and we can discuss how to proceed.

[philkra]

how are you running your tests? as suggested with the artisan command?

[dstepe]

We run our tests with phpunit from the command line. I am unclear what option is available to use artisan for running tests. The Laravel documentation, and the artisan help text, only mention make:test to create new test classes.

https://laravel.com/docs/5.7/testing#creating-and-running-tests

The documentation states explicitly "To run your tests, execute the phpunit command from your terminal".

Can you share how you run your tests? And to be clear, unit tests are not the issue. This is a problem with features tests which exercise Laravel routes, including middleware. A fresh Laravel application includes the CreatesApplication trait that is used by the abstract TestCase class which all tests should inherit from. The trait has one method:

    public function createApplication()
    {
        $app = require __DIR__.'/../bootstrap/app.php';

        $app->make(Kernel::class)->bootstrap();

        return $app;
    }

This method bootstraps the application for the test, but does not run either artisan or the web index.php file where the LARAVEL_START constant would be set.

I just started a fresh app and included the module to confirm all of this. The example feature test failed first because APM was active. After setting active to false, the test failed due to the undefined constant. I ran the tests with vendor/bin/phpunit tests/Feature/ so please help me understand if there is way to use artisan for this which I am not aware of.

Regardless of test running methodology, I think it is worth discontinuing the use of that constant. I have been unable to find any usage of that constant throughout the Laravel project. The framework authors are very careful about abstracting away implementation details and the fact that they do not provide a method to access this value indicates they do not intend for it to be readily available. I think the greater risk as continued reliance directly on the constant. Second, the REQUEST_TIME_FLOAT provided by PHP is a more accurate value for the purpose of application performance metrics. That value would be set without waiting for any framework code to be executed.

[philkra]

I ran the tests with vendor/bin/phpunit tests/Feature/ so please help me understand if there is way to use artisan for this which I am not aware of.

Ok, I got it! You need the bootstrap Laravel's "stuff" when you run the phpunit tests. I would highly recommend a phpunit.xml..

the same issue: https://github.com/barryvdh/laravel-debugbar/issues/15

[philkra]

This article could help as well: https://symfony.com/doc/current/testing/bootstrap.html

[dstepe]

I also found the DebugBar issue you referenced. However, if you look in the latest code for that project, they no longer use LARAVEL_START. They are now using the REQUEST_TIME_FLOAT, most likely for many of the same reasons we are discussing.

https://github.com/barryvdh/laravel-debugbar/commit/0de8f95e68c290cdfff9f4a34f35e4ff143512ec#diff-1d1c4a392b09331ac7d35038d7599b5d

I believe making this change is consistent with the framework expectations in addition to preventing the need for a customized bootstrap process for testing.

[philkra]

I was giving this another round and can't think of a reason why this would break stuff..let's go :)

[philkra]

this can be closed now due the merged PR #34