Skip to content

CI: php.yml (Valgrind + test matrix) not triggered on valkey-glide submodule bumps due to paths filter #330

Description

Summary

The PHP CI workflow (.github/workflows/php.yml), which contains the
Valgrind job and the full PHP test matrix, is gated by a paths: filter on
both push and pull_request. That filter does not match a submodule-pointer
bump
of valkey-glide, nor the top-level utils/ tooling. As a result, PRs
that update the glide-core submodule can merge without the Valgrind job (or the
php.yml matrix) ever running
, letting glide-core regressions slip in
unexercised.

Evidence

PR #325 ("Update submodule glide core") changed only:

  • utils/patch_proto_and_rust.py
  • valkey-glide (submodule pointer bump to the Valkey 9.0 core)

gh pr checks 325 shows no Valgrind job ran on that PR. The checks that did
run/pass came from other workflows; php.yml was skipped entirely.

The Valgrind leak-gate failures now tracked in #328 (glide-core global
possibly lost allocations) were introduced with that 9.0 core bump but only
surfaced later on PR #321, which is the first PR since the bump to touch a
php.yml-triggering path (*.c / *.h / *.php / tests/**).

Root cause (path filter)

php.yml watches specific submodule subpaths, e.g.:

valkey-glide/glide-core/src/**
valkey-glide/glide-core/redis-rs/redis/src/**
valkey-glide/ffi/src/**
valkey-glide/utils/cluster_manager.py

A submodule-pointer update is recorded as a change to the valkey-glide gitlink
(a single tree entry), which does not expand into the individual file paths
inside the new submodule commit. So none of the valkey-glide/.../** globs
match, and the workflow is skipped. Likewise, the top-level utils/** directory
is not in the trigger list (only valkey-glide/utils/cluster_manager.py is).

Proposed fix

  • Add trigger paths so glide-core bumps and tooling changes run php.yml, e.g.:
    • valkey-glide (the submodule gitlink itself)
    • utils/**
  • Alternatively, treat submodule updates as always-trigger for php.yml (or run
    the Valgrind/test matrix on a schedule against main as a safety net).
  • Verify by opening a test PR that only bumps the submodule and confirming the
    Valgrind job is triggered.

Priority

This should be addressed before #328: without CI actually running the
Valgrind job on submodule bumps, regressions like the one in #328 will continue
to land undetected regardless of any suppression work.

Acceptance criteria

  • A submodule-only bump of valkey-glide triggers php.yml (Valgrind +
    test matrix).
  • Top-level utils/** changes trigger php.yml.
  • Confirmed with a test PR that the Valgrind job runs.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions