Expand appsec laravel integration testing to all versions by estringana · Pull Request #4277 · DataDog/dd-trace-php

estringana · 2026-10-06T08:29:19Z

Description

Following #4268 this pr expand tests on Laravel

Reviewer checklist

Test coverage seems ok.
Appropriate labels assigned.

datadog-datadog-prod-us1 · 2026-10-06T08:36:58Z

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
• Patch Coverage: 100.00%
• Overall Coverage: 68.38% (-0.04%)

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: 59e2f36 | Docs | View more details | Give us feedback!}

chatgpt-codex-connector · 2026-10-07T09:54:34Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-07T10:02:54.730196Z	`3aca3a6`	Draft marked ready
🔒 Security Review	✅ Completed 2026-10-07T09:58:38.049406Z	`3aca3a6`	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Copilot

Copilot review overview

🟡 Changes recommended

Supported older Laravel versions remain uncovered, and endpoint/debug-build coverage is reduced.

Review effort: Balanced
Findings: 2 · 1

Open (3)

Add missing Laravel 4.2, 5.7, and 5.8 test coverage · New
Preserve Laravel coverage for debug extension builds · New
Correct nonexistent composer.lock-php84 fixture reference · New

What changed in this PR

Expands Laravel AppSec integration testing by extracting shared tests and adding coverage for Laravel 9.x–12.x.

Changes:

Introduces a shared Laravel AppSec test suite and version-specific containers.
Adds authentication, endpoint, telemetry, and SQLite fixture setup.
Seeds users consistently for authentication-event testing.

File	Description
`tests/Frameworks/Laravel/Version_9_x/routes/web.php`	Adds endpoint-test routes.
`tests/Frameworks/Laravel/Version_9_x/public/outside_of_framework.php`	Exposes endpoint-collection state.
`tests/Frameworks/Laravel/Version_9_x/docker-init.sh`	Initializes the test fixture.
`tests/Frameworks/Laravel/Version_9_x/database/seeders/DatabaseSeeder.php`	Seeds the authentication user.
`tests/Frameworks/Laravel/Version_9_x/app/Http/Controllers/LoginTestController.php`	Supports failed-password testing.
`tests/Frameworks/Laravel/Version_10_x/routes/web.php`	Adds endpoint-test routes.
`tests/Frameworks/Laravel/Version_10_x/public/outside_of_framework.php`	Exposes endpoint-collection state.
`tests/Frameworks/Laravel/Version_10_x/docker-init.sh`	Initializes the test fixture.
`tests/Frameworks/Laravel/Version_10_x/database/seeders/DatabaseSeeder.php`	Seeds the authentication user.
`tests/Frameworks/Laravel/Version_10_x/app/Http/Controllers/LoginTestController.php`	Supports failed-password testing.
`tests/Frameworks/Laravel/Version_11_x/routes/web.php`	Adds authentication and endpoint routes.
`tests/Frameworks/Laravel/Version_11_x/public/outside_of_framework.php`	Exposes endpoint-collection state.
`tests/Frameworks/Laravel/Version_11_x/docker-init.sh`	Initializes the test fixture.
`tests/Frameworks/Laravel/Version_11_x/database/seeders/DatabaseSeeder.php`	Seeds the authentication user.
`tests/Frameworks/Laravel/Version_11_x/app/Http/Controllers/LoginTestController.php`	Implements authentication test actions.
`tests/Frameworks/Laravel/Latest/routes/web.php`	Adds authentication and endpoint routes.
`tests/Frameworks/Laravel/Latest/public/outside_of_framework.php`	Exposes endpoint-collection state.
`tests/Frameworks/Laravel/Latest/docker-init.sh`	Initializes the latest fixture.
`tests/Frameworks/Laravel/Latest/database/seeders/DatabaseSeeder.php`	Seeds the authentication user.
`tests/Frameworks/Laravel/Latest/app/Http/Controllers/LoginTestController.php`	Implements authentication test actions.
`appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/LaravelLatestTests.groovy`	Configures latest-Laravel tests.
`appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/Laravel9xTests.groovy`	Configures Laravel 9 tests.
`appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/Laravel8xTests.groovy`	Migrates Laravel 8 to shared tests.
`appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/Laravel11xTests.groovy`	Configures Laravel 11 tests.
`appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/Laravel10xTests.groovy`	Configures Laravel 10 tests.
`appsec/tests/integration/src/test/groovy/com/datadog/appsec/php/integration/AbstractLaravelAppsecTests.groovy`	Defines reusable Laravel AppSec tests.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3aca3a6d55

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

pr-commenter · 2026-10-07T16:46:24Z

Benchmarks [ tracer ]

Benchmark execution time: 2026-10-08 13:46:34

Comparing candidate commit 59e2f36 in PR branch estringana/test-more-laravel-versions with baseline commit 2c0c767 in branch master.

📊 Benchmarking dashboard

Found 1 performance improvements and 0 performance regressions! Performance is the same for 193 metrics, 0 unstable metrics.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

🟩 = significantly better candidate vs. baseline
🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

scenario:SymfonyBench/benchSymfonyDdprof

🟩 execution_time [-1325.216µs; -637.424µs] or [-11.814%; -5.683%]

cataphract · 2026-10-08T10:04:26Z

+if [[ -f composer.lock ]]; then
+    composer install --no-dev --no-scripts
+else
+    composer update --no-dev --no-scripts
+fi


It should always be there, no?

cataphract · 2026-10-08T10:10:54Z

+    /** Expected total endpoints count; return negative to skip exact check. */
+    int expectedEndpointCount() { -1 }


Just make it a bool like allowUnenumeratedRoutes. expectedEndpoints already implicitly carries a count

cataphract · 2026-10-08T10:11:58Z

+    /** Expected total endpoints count; return negative to skip exact check. */
+    int expectedEndpointCount() { -1 }
+
+    /**
+     * Status code the fixture's /login/signup endpoint returns.
+     * Laravel 8.x returns 200 explicitly; 9.x+ scaffolded `register()` redirects
+     * (`return redirect('/simple')`) which produces 302.
+     */
+    int expectedSignupStatus() { 200 }
+
+    /** List of {@code [path, method, resourceName]} entries that must be present. */
+    List<List<String>> expectedEndpoints() { [] }


nit: these would be nicer is they were named is* (for bools) or get* (otherwise) because then in groovy you could use property syntax (expectedEndpoints rather than expectedEndpoints())

cataphract · 2026-10-08T10:13:45Z

+    // Not a @Test on purpose — pre-existing behaviour of Laravel8xTests. The
+    // tracer bookkeeping is per-request, so re-asserting the flag on a
+    // subsequent request to outside_of_framework.php was unreliable.
+    @Order(3)
+    void 'Endpoints are collected after the first request to framework'() {
+        HttpRequest req = container.buildReq('/outside_of_framework.php').GET().build()
+        container.traceFromRequest(req, ofString()) { HttpResponse<String> re ->
+            assert re.statusCode() == 200
+            assert re.body().contains('are_endpoints_collected: true')
+        }
+    }


What does this mean "tracer bookkeeping is per-request" ??

Either way, it seems the test should be fixed.

Is this introduced by this PR? This is just moved from somewhere else

Expand appsec laravel integration testing to all versions

7e13780

estringana added 3 commits October 6, 2026 11:37

Fix pipeline

c664367

Fix

b0f15fc

Fix login

3aca3a6

estringana requested a balanced review from Copilot October 7, 2026 09:53

estringana marked this pull request as ready for review October 7, 2026 09:54

estringana requested review from a team as code owners October 7, 2026 09:54

Copilot started reviewing on behalf of estringana October 7, 2026 09:54 View session

Copilot AI reviewed Oct 7, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Oct 7, 2026

View reviewed changes

Comment thread ...gration/src/test/groovy/com/datadog/appsec/php/integration/AbstractLaravelAppsecTests.groovy Outdated

Comment thread tests/Frameworks/Laravel/Latest/composer.lock Outdated

Comment thread tests/Frameworks/Laravel/Version_10_x/composer.lock Outdated

Address comments

21c945f

cataphract reviewed Oct 8, 2026

View reviewed changes

estringana added 2 commits October 8, 2026 12:55

Adress comments

000af8f

Fix tests

59e2f36

		/** Expected total endpoints count; return negative to skip exact check. */
		int expectedEndpointCount() { -1 }

Conversation

estringana commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Reviewer checklist

Uh oh!

datadog-datadog-prod-us1 Bot commented Oct 6, 2026 • edited by datadog-datadog-prod-us1-2 Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pr-commenter Bot commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks [ tracer ]

Explanation

More details about the CI and significant changes

scenario:SymfonyBench/benchSymfonyDdprof

Uh oh!

cataphract Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

cataphract Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

cataphract Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

cataphract Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

estringana Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

estringana commented Oct 6, 2026 •

edited

Loading

datadog-datadog-prod-us1 Bot commented Oct 6, 2026 •

edited by datadog-datadog-prod-us1-2 Bot

Loading

chatgpt-codex-connector Bot commented Oct 7, 2026 •

edited

Loading

pr-commenter Bot commented Oct 7, 2026 •

edited

Loading