Skip to content

Commit 7c52ffc

Browse files
committed
Fixed tests
1 parent f2da390 commit 7c52ffc

File tree

6 files changed

+41
-14
lines changed

6 files changed

+41
-14
lines changed

agent/php/ElasticApm/Impl/AutoInstrument/TransactionForExtensionRequest.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,12 @@ private function setTxPropsBasedOnHttpRequestData(TransactionInterface $tx): voi
345345

346346
private function beforeHttpEnd(TransactionInterface $tx): void
347347
{
348+
($loggerProxy = $this->logger->ifDebugLevelEnabled(__LINE__, __FUNCTION__))
349+
&& $loggerProxy->log(
350+
'Entered',
351+
['tx' => ['result' => $tx->getResult(), 'outcome' => $tx->getOutcome()], 'lastThrown' => $this->lastThrown, "ini_get('display_errors')" => ini_get('display_errors')]
352+
);
353+
348354
if ($tx->getResult() === null) {
349355
$this->discoverHttpResult($tx);
350356
}
@@ -449,6 +455,9 @@ public function onNotCaughtThrowableCapturedByPhpPart(Throwable $thrown): void
449455
if ($this->prevExceptionHandler !== null) {
450456
($this->prevExceptionHandler)($thrown);
451457
}
458+
459+
// Re-throw because otherwise HTTP status will be 200 instead of 500
460+
throw $thrown;
452461
}
453462

454463
public function onShutdown(): void

tests/ElasticApmTests/ComponentTests/CurlAutoInstrumentationTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ public static function appCodeServer(): void
9696

9797
public function testLocalClientServer(): void
9898
{
99-
// OLD TODO: Sergey Kleyman: Implement: CurlAutoInstrumentationTest::testLocalClientServer
99+
// OLD TODO: Implement: CurlAutoInstrumentationTest::testLocalClientServer
100100
if (PHP_MAJOR_VERSION < 9) {
101101
self::dummyAssert();
102102
return;
@@ -125,7 +125,7 @@ function (AppCodeRequestParams $reqParams) use ($serverAppCode): void {
125125

126126
public function testLocalClientExternalServer(): void
127127
{
128-
// OLD TODO: Sergey Kleyman: Implement: CurlAutoInstrumentationTest::testLocalClientExternalServer
128+
// OLD TODO: Implement: CurlAutoInstrumentationTest::testLocalClientExternalServer
129129
self::dummyAssert();
130130
}
131131
}

tests/ElasticApmTests/ComponentTests/ErrorComponentTest.php

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
use Elastic\Apm\ElasticApm;
2727
use Elastic\Apm\Impl\Config\AllOptionsMetadata;
2828
use Elastic\Apm\Impl\Config\OptionNames;
29+
use Elastic\Apm\Impl\Constants;
2930
use Elastic\Apm\Impl\StackTraceFrame;
3031
use Elastic\Apm\Impl\Util\ArrayUtil;
3132
use Elastic\Apm\Impl\Util\ClassNameUtil;
@@ -193,10 +194,10 @@ public function dataProviderForTestPhpErrorUndefinedVariable(): iterable
193194
{
194195
$result = (new DataProviderForTestBuilder())
195196
->addBoolKeyedDimensionAllValuesCombinable(self::INCLUDE_IN_ERROR_REPORTING_KEY)
196-
->addBoolKeyedDimensionAllValuesCombinable(OptionNames::CAPTURE_ERRORS)
197-
->addBoolKeyedDimensionAllValuesCombinable(OptionNames::CAPTURE_ERRORS_WITH_PHP_PART)
198-
->addKeyedDimensionAllValuesCombinable(OptionNames::CAPTURE_EXCEPTIONS, [null, false, true])
199-
->addKeyedDimensionAllValuesCombinable(OptionNames::DEV_INTERNAL_CAPTURE_ERRORS_ONLY_TO_LOG, [false, true])
197+
->addAgentBoolConfigOptionKeyedDimensionAllValuesCombinable(OptionNames::CAPTURE_ERRORS)
198+
->addAgentBoolConfigOptionKeyedDimensionAllValuesCombinable(OptionNames::CAPTURE_ERRORS_WITH_PHP_PART)
199+
->addAgentNullableBoolConfigOptionKeyedDimensionAllValuesCombinable(OptionNames::CAPTURE_EXCEPTIONS)
200+
->addAgentBoolConfigOptionKeyedDimensionAllValuesCombinable(OptionNames::DEV_INTERNAL_CAPTURE_ERRORS_ONLY_TO_LOG)
200201
->build();
201202

202203
return DataProviderForTestBuilder::convertEachDataSetToMixedMap(self::adaptKeyValueToSmoke($result));
@@ -226,6 +227,7 @@ function (AppCodeRequestParams $appCodeRequestParams) use ($testArgs): void {
226227
$expectedErrorCount = $isErrorExpected ? 1 : 0;
227228
$dataFromAgent = $testCaseHandle->waitForDataFromAgent((new ExpectedEventCounts())->transactions(1)->errors($expectedErrorCount));
228229
$dbgCtx->add(compact('dataFromAgent'));
230+
229231
self::assertCount($expectedErrorCount, $dataFromAgent->idToError);
230232
if (!$isErrorExpected) {
231233
return;
@@ -302,6 +304,14 @@ function () use ($testArgs): void {
302304

303305
public static function appCodeForTestPhpErrorUncaughtExceptionWrapper(bool $justReturnLineNumber = false): int
304306
{
307+
if (!$justReturnLineNumber) {
308+
/**
309+
* We set display_errors to 0 to ensure HTTP status 500 is returned on PHP error.
310+
* For more details see https://bugs.php.net/bug.php?id=50921
311+
*/
312+
ini_set('display_errors', '0');
313+
}
314+
305315
return $justReturnLineNumber ? __LINE__ : appCodeForTestPhpErrorUncaughtException();
306316
}
307317

@@ -337,12 +347,7 @@ private function implTestPhpErrorUncaughtException(MixedMap $testArgs): void
337347
AppCodeTarget::asRouted([__CLASS__, 'appCodeForTestPhpErrorUncaughtExceptionWrapper']),
338348
function (AppCodeRequestParams $appCodeRequestParams): void {
339349
if ($appCodeRequestParams instanceof HttpAppCodeRequestParams) {
340-
/**
341-
* It seems it depends on 'display_errors' whether 200 or 500 is returned on PHP error.
342-
* We ignore HTTP status since it's not the main point of the test case.
343-
* For more details see https://bugs.php.net/bug.php?id=50921
344-
*/
345-
$appCodeRequestParams->expectedHttpResponseStatusCode = null;
350+
$appCodeRequestParams->expectedHttpResponseStatusCode = HttpConstantsForTests::STATUS_INTERNAL_SERVER_ERROR;
346351
}
347352
}
348353
);
@@ -364,6 +369,11 @@ function (AppCodeRequestParams $appCodeRequestParams): void {
364369
$expectedErrorCount = $isErrorExpected ? 1 : 0;
365370
$dataFromAgent = $testCaseHandle->waitForDataFromAgent((new ExpectedEventCounts())->transactions(1)->errors($expectedErrorCount));
366371
$dbgCtx->add(compact('dataFromAgent'));
372+
373+
if (self::isMainAppCodeHostHttp()) {
374+
self::assertSame(Constants::OUTCOME_FAILURE, $dataFromAgent->singleTransaction()->outcome);
375+
}
376+
367377
self::assertCount($expectedErrorCount, $dataFromAgent->idToError);
368378
if (!$isErrorExpected) {
369379
return;
@@ -468,6 +478,11 @@ function (AppCodeRequestParams $appCodeRequestParams): void {
468478
$expectedErrorCount = $isErrorExpected ? 1 : 0;
469479
$dataFromAgent = $testCaseHandle->waitForDataFromAgent((new ExpectedEventCounts())->transactions(1)->errors($expectedErrorCount));
470480
$dbgCtx->add(compact('dataFromAgent'));
481+
482+
if (self::isMainAppCodeHostHttp()) {
483+
self::assertSame(Constants::OUTCOME_FAILURE, $dataFromAgent->singleTransaction()->outcome);
484+
}
485+
471486
self::assertCount($expectedErrorCount, $dataFromAgent->idToError);
472487
if (!$isErrorExpected) {
473488
return;

tests/ElasticApmTests/ComponentTests/InferredSpansComponentTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ final class InferredSpansComponentTest extends ComponentTestCaseBase
7272
public function dataProviderForTestInferredSpans(): iterable
7373
{
7474
$result = (new DataProviderForTestBuilder())
75-
// OLD TODO: Sergey Kleyman: Implement: test with PROFILING_INFERRED_SPANS_ENABLED set to true
75+
// OLD TODO: Implement: test with PROFILING_INFERRED_SPANS_ENABLED set to true
7676
// ->addBoolKeyedDimensionOnlyFirstValueCombinable(self::IS_INFERRED_SPANS_ENABLED_KEY)
7777
->addKeyedDimensionOnlyFirstValueCombinable(self::IS_INFERRED_SPANS_ENABLED_KEY, DataProviderForTestBuilder::isLongRunMode() ? [true, false] : [false])
7878
->addBoolKeyedDimensionOnlyFirstValueCombinable(self::IS_TRANSACTION_SAMPLED_KEY)

tests/ElasticApmTests/ComponentTests/Util/HttpAppCodeHostHandle.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,9 @@ public function sendHttpRequest(AppCodeTarget $appCodeTarget, ?Closure $setParam
9999
);
100100
$this->afterAppCodeInvocation($appCodeInvocation);
101101

102+
($loggerProxy = $localLogger->ifDebugLevelEnabled(__LINE__, __FUNCTION__))
103+
&& $loggerProxy->log('HTTP response status code: ' . $response->getStatusCode());
104+
102105
if ($requestParams->expectedHttpResponseStatusCode !== null) {
103106
TestCase::assertSame(
104107
$requestParams->expectedHttpResponseStatusCode,

tests/ElasticApmTests/TestsSharedCode/TransactionMaxSpansTest/SharedCode.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ final class SharedCode
4747
*/
4848
public static function configTransactionMaxSpansVariants(int $testingDepth): iterable
4949
{
50-
// OLD TODO: Sergey Kleyman: UNCOMMENT and remove the "if" block below
50+
// OLD TODO: UNCOMMENT and remove the "if" block below
5151
// yield null;
5252
if ($testingDepth >= self::TESTING_DEPTH_1) {
5353
yield null;

0 commit comments

Comments
 (0)