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

Open Beginner friendly
#330 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
82/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
git, github-actions, php
Domain
ci-cd, testing-qa

Research direction

Start with .github/workflows/php.yml and inspect its push and pull_request paths filters, then compare them with the valkey-glide submodule entry and top-level utils/** changes described here. Update the triggers so both change types run the Valgrind job and PHP test matrix, then verify with a test PR that only bumps the submodule and confirm php.yml runs.

Written by the indexing model from the issue text.

Description

bug

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.
Dominant language
PHP
Stars
36
Forks
12
Avg merge
2d 2h
Merged PRs (30d)
10

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from valkey-io/valkey-glide-php

All issues in valkey-io/valkey-glide-php

Similar issues

More PHP issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.