Skip to content

test(debugger): validate probe creation in expression language tests#6129

Merged
watson merged 1 commit intomainfrom
watson/fail-test
Feb 5, 2026
Merged

test(debugger): validate probe creation in expression language tests#6129
watson merged 1 commit intomainfrom
watson/fail-test

Conversation

@watson
Copy link
Contributor

@watson watson commented Jan 27, 2026

Motivation

The test_expression_language_contextual_variables test was XPASS'ing for Node.js

Changes

Modified the test to validate the actual feature implementation instead of explicitly failing:

  • Added assertion in _assert() to verify probes are successfully created before validation

The test now:

  • For languages with method probe support: Creates probes, validates they emit correctly, and checks @return and @duration values
  • For Node.js: Naturally fails when probes cannot be created with a clear error message: "Expected probes to be created for validation. Check if the language supports the required probe type for this test."

Workflow

  1. ⚠️ Create your PR as draft ⚠️
  2. Work on you PR until the CI passes
  3. Mark it as ready for review
    • Test logic is modified? -> Get a review from RFC owner.
    • Framework is modified, or non obvious usage of it -> get a review from R&P team

🚀 Once your PR is reviewed and the CI green, you can merge it!

🛟 #apm-shared-testing 🛟

Reviewer checklist

  • Anything but tests/ or manifests/ is modified ? I have the approval from R&P team
  • A docker base image is modified?
    • the relevant build-XXX-image label is present
  • A scenario is added, removed or renamed?

Copy link
Contributor Author

watson commented Jan 27, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions
Copy link
Contributor

CODEOWNERS have been resolved as:

tests/debugger/test_debugger_expression_language.py                     @DataDog/debugger @DataDog/system-tests-core

@watson watson marked this pull request as ready for review January 30, 2026 15:23
@watson watson requested review from a team as code owners January 30, 2026 15:23
Copy link
Collaborator

@nccatoni nccatoni left a comment

Choose a reason for hiding this comment

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

Ideally this test should check that the feature is implemented. I see that @tylfin created this test. Would it be possible to add such an assertion ?

Add assertion to verify probes are created before validation.
This allows the test to fail naturally with a meaningful error when
probes cannot be created (e.g., Node.js lacks method probe support).
@watson watson changed the title [nodejs] fail contextual variable test for Node.js test(debugger): validate probe creation in expression language tests Feb 5, 2026
@watson
Copy link
Contributor Author

watson commented Feb 5, 2026

@nccatoni good point. I've updated the PR to do that now

@watson watson requested a review from nccatoni February 5, 2026 13:16
Copy link
Collaborator

@nccatoni nccatoni left a comment

Choose a reason for hiding this comment

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

Thank you for the changes. LGTM, you might also want a review from someone familiar with the feature

@watson watson enabled auto-merge (squash) February 5, 2026 13:34
@watson watson merged commit 9ef6e89 into main Feb 5, 2026
801 of 803 checks passed
@watson watson deleted the watson/fail-test branch February 5, 2026 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants