Skip to content

Add TemporaryRequestContext unit tests - #11798

Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 2 commits into
masterfrom
clara.poncet/lambda-appsec-temporary-request-context-tests
Jun 30, 2026
Merged

Add TemporaryRequestContext unit tests#11798
gh-worker-dd-mergequeue-cf854d[bot] merged 2 commits into
masterfrom
clara.poncet/lambda-appsec-temporary-request-context-tests

Conversation

@claponcet

@claponcetclaponcet commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

What Does This Do

Adds unit tests for the TemporaryRequestContext private static class inside LambdaAppSecHandler, covering:

  • getData() returns the stored AppSec context for the APPSEC slot and null for other slots
  • All no-op methods (getTraceSegment, getBlockResponseFunction, getOrCreateMetaStructTop, getClientIpAddressData, setters, close) return expected defaults and do not throw

Since TemporaryRequestContext is private, tests capture an instance by intercepting the requestMethodUriRaw callback during a processRequestStart invocation with a mocked tracer.

Motivation

Improve test coverage for TemporaryRequestContext.

Additional Notes

tag: no release note

Contributor Checklist

Jira ticket: [PROJ-IDENT]

claponcetand others added 2 commits June 30, 2026 10:11
@claponcetclaponcet added comp: tooling Build & Tooling type: bug fix Bug fix tag: no release notes Changes to exclude from release notes labels Jun 30, 2026
@claponcet
claponcet marked this pull request as ready for review June 30, 2026 08:19
@claponcet
claponcet requested a review from a team as a code ownerJune 30, 2026 08:19
@claponcet
claponcet requested a review from mhliddJune 30, 2026 08:19
@dd-octo-stsdd-octo-stsBot added the tag: ai generated Largely based on code generated by an AI or LLM label Jun 30, 2026
@claponcet
claponcet enabled auto-merge June 30, 2026 08:24

assertNotNull(ctx);
assertEquals(mockAppSecContext, ctx.getData(RequestContextSlot.APPSEC));
assertNull(ctx.getData(RequestContextSlot.CI_VISIBILITY));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: consider also testing a slot beyond CI_VISIBILITY (e.g. IAST) and the edge case where appSecContext is null — in that case getData(APPSEC) should also return null, which is currently not covered.

}

@Test
void temporaryRequestContextNoOpMethodsReturnExpectedDefaults() throws Exception {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: throws Exception is unnecessary here — none of the no-op methods actually throw a checked exception. The comment "verify no-op methods don't throw" makes the intent clear, but the throws clause sends the opposite signal. Removing it makes the method signature self-documenting.

@dd-octo-sts

Copy link
Copy Markdown
Contributor

🟢 Java Benchmark SLOs — All performance SLOs passed

SuiteStatus
Startup🟢 pass

SLO thresholds are defined here based on automatically generated metrics. A warning is raised when results are within 5% of the threshold.

PR vs. master results
ScenarioCandidatemasterΔ (95% CI of mean)
startup:insecure-bank:iast:Agent14.85 s14.68 s[+0.4%; +2.0%] (maybe worse)
startup:insecure-bank:tracing:Agent13.60 s13.72 s[-1.7%; +0.1%] (no difference)
startup:petclinic:appsec:Agent16.82 s16.76 s[-0.6%; +1.4%] (no difference)
startup:petclinic:iast:Agent16.31 s16.90 s[-7.8%; +0.9%] (no difference)
startup:petclinic:profiling:Agent16.82 s16.87 s[-1.5%; +0.9%] (no difference)
startup:petclinic:sca:Agent16.87 s16.62 s[+0.6%; +2.5%] (maybe worse)
startup:petclinic:tracing:Agent16.02 s15.56 s[-1.4%; +7.3%] (no difference)

Commit:a25135a7 · CI Pipeline · Benchmarking Platform UI


Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion.

@jandro996jandro996 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! just minor comments

@claponcet
claponcet added this pull request to the merge queueJun 30, 2026
@dd-octo-sts

Copy link
Copy Markdown
Contributor

/merge

@gh-worker-devflow-routing-ef8351

gh-worker-devflow-routing-ef8351Bot commented Jun 30, 2026

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2026-06-30 08:55:00 UTC ℹ️ Start processing command /merge


2026-06-30 08:55:05 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in master is approximately 2h (p90).


2026-06-30 09:54:53 UTC ℹ️ MergeQueue: This merge request was merged

@github-merge-queue
github-merge-queueBot removed this pull request from the merge queue due to failed status checks Jun 30, 2026
@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854dBot merged commit d9b0bfe into masterJun 30, 2026
599 of 601 checks passed
@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854dBot deleted the clara.poncet/lambda-appsec-temporary-request-context-tests branch June 30, 2026 09:54
@github-actionsgithub-actionsBot added this to the 1.64.0 milestone Jun 30, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp: toolingBuild & Toolingtag: ai generatedLargely based on code generated by an AI or LLMtag: no release notesChanges to exclude from release notestype: bug fixBug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@claponcet@amarziali@jandro996