Skip to content

ci: run the whole test suite - #11

Open
somethingwithproof wants to merge 2 commits into
php:masterfrom
somethingwithproof:ci/run-full-test-suite
Open

ci: run the whole test suite#11
somethingwithproof wants to merge 2 commits into
php:masterfrom
somethingwithproof:ci/run-full-test-suite

Conversation

@somethingwithproof

@somethingwithproof somethingwithproof commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

16 of the 22 tests are gated on fixtures under tests/data/ that nothing ever builds, so their SKIPIF blocks skip them and CI has been reporting green on 6 tests.

Adding make -C tests/data all (and rrdtool to the apt list, which is what that Makefile shells out to) brings all 22 into the run. Two then fail, both stale expectations rather than regressions:

  • rrd_016 ends in a trailing %s, which needs at least one character and never gets one.
  • rrd_017 expects an extra trailing sample and end=920808300. librrd returns end=920808000 and 12 rows on 1.7.2 (Debian 13, Ubuntu 24.04) and on 1.11.0 (Homebrew) alike.

.gitignore picks up the generated fixtures and the run-tests output so the suite stops dirtying the tree.

Validation

phpize && ./configure --with-rrd && make && make -C tests/data all && make test, on php:8.4-cli with librrd 1.7.2 and on macOS with librrd 1.11.0: 22/22 pass, REPORT_EXIT_STATUS=1 exits 0.


Stack order. This is part of a series, each building on the one before. Merge order:
#11 (test suite) → #12 (memory safety) → #13 (open_basedir, value semantics) → #14 (php-src master) → #15 (librrd argv) → #16 (argument builder) → #17 (autoconf). Commits below this PR's own belong to the ones underneath it.

Without tests/data the SKIPIF blocks skip 16 of the 22 tests, so CI reported green while most of the suite never ran.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Both were written against an older librrd and fail against 1.7.2 and 1.11.0 alike; they were never noticed because the fixtures were missing.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
@somethingwithproof

somethingwithproof commented Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

@remicollet Thanks for the context on #18. I have split the follow-up work into a linear stack so each concern can be reviewed separately:\n\n#11 → #12#13#14#15#16#17
All current CI checks pass. #11 enables the full test suite and is the intended starting point. #12 is the highest-priority functional change because it fixes several memory-safety failures reachable from ordinary PHP; the later PRs cover behavioral consistency, compatibility, warnings, and cleanup.\n\nI will rebase the stack after #18 lands and remove its overlap with #14. I am also happy to consolidate or squash parts of the stack if that is easier to review. When you have time next week, starting with #11 would keep each subsequent diff manageable.

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.

1 participant