Skip to content

[docs] Fix false claim that specific manifest rules override directory rules - #7446

Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 1 commit into
mainfrom
dario.castane/festive-herschel-a4bf79
Aug 5, 2026
Merged

[docs] Fix false claim that specific manifest rules override directory rules#7446
gh-worker-dd-mergequeue-cf854d[bot] merged 1 commit into
mainfrom
dario.castane/festive-herschel-a4bf79

Conversation

@darccio

@darccio darccio commented Aug 4, 2026

Copy link
Copy Markdown
Member

Motivation

docs/edit/manifest.md contained a factually incorrect statement about manifest rule
resolution, in the section people read precisely when they are trying to activate a test:

Directory-Level Rules

Apply rules to entire directories. More specific rules override directory rules

Manifest declarations accumulate — they never override. Manifest.get_declarations
(utils/manifest/_internal/core.py) appends the declarations of every rule whose
path-segment prefix matches the nodeid (match_rule in rule.py is a pure prefix match),
and conftest.py calls add_pytest_marker once per collected declaration. Nothing anywhere
picks a "most specific" winner.

Worse, the example given to illustrate the claim demonstrates the opposite. Verified with
that exact snippet: at python 3.0.0, the nodeid
tests/appsec/iast/sink/test_specific.py::Test_X::test_y still resolves to
missing_feature, inherited from the directory rule, even though it satisfies the more
specific v2.0.0 entry. Following the doc as written leaves you with a test you believe is
enabled and that is silently still disabled.

Changes

Docs only — no framework or test behaviour is touched.

Split the old section into ### Directory-Level Rules (prefix matching only) and a new
### How Entries Combine, which:

  • States that declarations from every matching level (directory, file, class, method)
    accumulate, and that a narrower entry can only add restrictions.
  • Explains why, which turns the rule from an arbitrary gotcha into something
    derivable: no entry ever declares a test enabled. process_inline (parser.py) turns
    v2.0.0 into a condition with excluded_component_version: >=2.0.0 carrying a
    missing_feature declaration, so a satisfied version entry emits nothing at all. Only
    disabling declarations exist — hence there is nothing for a narrower entry to override a
    broader one with.
  • Keeps the original YAML as the worked example, re-annotated with what actually happens.
  • Adds the two ways to genuinely enable a test that a broader entry disables — narrowing
    that entry by version (component_version) or by weblog (excluded_weblog/weblog) —
    plus a note to restructure the broader entry when neither fits.
  • Documents the corollary that weblog lists need not be mirrored onto narrower entries:
    when a broader weblog_declaration already gates unlisted weblogs through '*', a bare
    version entry below it is safe.

Notes for the reviewer

Every YAML block in the new section was executed against a throwaway manifest directory
via Manifest(components, weblog, path=...) before being written down — 13 cases in total,
including the negative ones (unlisted weblogs such as fastapi/express4 staying gated by
'*', and versions below the narrower entry still being gated by it). All matched the
documented behaviour.

I also grepped docs/ for other statements about override/precedence in manifest
resolution; the claim was confined to this one section, and docs/edit/enable-test.md does
not repeat it.

./format.sh passes. Two failures show up on a first local run and are pre-existing and
environmental
, not from this change (both already known on this machine):

  • yamlfmt reformatted manifests/python.yml (85 lines of *django:*django :).
    Cause: format.sh pins yamlfmt 0.16.0 but only installs it when which yamlfmt finds
    nothing, so a newer global v0.20.0 shadows it. Reverted; confirmed the pinned 0.16.0
    leaves manifests/ untouched and lint-clean, so nothing of it is in this PR.
  • utils/scripts/shellcheck.sh dies with line 74: @: unbound variable under macOS bash
    3.2 (empty "${@}" with set -u). Passes under bash 5, as in CI.

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 teamapplies here: this changes docs/, so R&P approval is required.
  • A docker base image is modified? — no
    • the relevant build-XXX-image label is present
  • A scenario is added, removed or renamed? — no

🤖 Generated with Claude Code

…y rules

`docs/edit/manifest.md` stated "More specific rules override directory
rules" and illustrated it with an example that does the opposite of what
the surrounding text claims.

Manifest declarations accumulate. `Manifest.get_declarations` appends the
declarations of every rule whose path-segment prefix matches the nodeid
(`match_rule` is a pure prefix match), and `conftest.py` adds one pytest
marker per collected declaration. Nothing picks a "most specific" winner.

The root cause is that no entry ever declares a test enabled: a version
entry like `v2.0.0` is turned by `process_inline` into a condition with
`excluded_component_version: >=2.0.0` carrying a `missing_feature`
declaration, so a *satisfied* version entry emits nothing at all. Only
disabling declarations exist, which is why a narrower entry has nothing
to override a broader one with.

Verified with the doc's own example: at python 3.0.0, the nodeid
tests/appsec/iast/sink/test_specific.py::Test_X::test_y still resolves to
missing_feature inherited from the directory rule, despite satisfying the
more specific v2.0.0 entry.

Replace the misleading example with the same YAML annotated with what
actually happens, and document the ways to genuinely enable a test that a
broader entry disables: narrowing that entry with `component_version`,
`excluded_weblog`/`weblog` or `weblog_declaration`, or restructuring it.
Also document the corollary that weblog lists need not be mirrored onto
narrower entries when a broader `weblog_declaration` already gates
unlisted weblogs via `'*'`.

Every YAML block in the new section was executed against a throwaway
manifest directory to confirm it behaves as documented.

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

CODEOWNERS have been resolved as:

docs/edit/manifest.md                                                   @DataDog/system-tests-core

@darccio darccio added the ai-generated The pull request includes a significant amount of AI-generated code label Aug 4, 2026
@darccio
darccio marked this pull request as ready for review August 4, 2026 14:56
@darccio
darccio requested a review from a team as a code owner August 4, 2026 14:56

@nccatoni nccatoni left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Great catch! Thank for the correction

@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854d Bot merged commit 10bffb2 into main Aug 5, 2026
16 checks passed
@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854d Bot deleted the dario.castane/festive-herschel-a4bf79 branch August 5, 2026 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-generated The pull request includes a significant amount of AI-generated code mergequeue-status: done

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants