Repository navigation
ci: count the Python bindings in python/src in the C++ coverage report - #31
Merged
Merged
Conversation
Build with clang coverage instrumentation through CFLAGS and CXXFLAGS. Both builds in build-macos run cmake from scratch and read them. The test step writes one profile for each process and instrumented binary. After the tests, merge the profiles and report line coverage for the C++ test binary and the installed Python extension and libraries. Write the totals to the job summary and upload the full report as an artifact. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
The first report counted the generated sources in the two build folders and the headers of the uv Python. Ignore every path under a build folder and the uv Python folder, so that the totals count only the mlx sources. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
The job summary now shows one table with Lines, Branches and Functions for all host C++ under mlx/ and for backend/metal, backend/cpu, backend/common, distributed and io, each against the 80% target. The target is reported only. The report now also leaves out the Metal shader source in mlx/backend/metal/kernels/, the CUDA backend and the vendored pocketfft header. The step reads each value from the llvm-cov TOTAL row by its column name and fails when a value is missing. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The kernels exclude dropped steel/conv/params.h, a header that host C++ includes and that has coverage records. The shader source has no records, so the exclude is removed. The text says "link" instead of "run", and a comment notes that value_of rejects the "-" of a column without counters. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The Python tests run the bindings through the installed wheel, but the report dropped python/src with an ignore pattern. The report now counts python/src in the total and shows it in its own row. The rows for mlx/ and its directories do not change. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ndings # Conflicts: # .github/workflows/fork-tests.yml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
The C++ coverage report of Fork Tests ignores
python/src, the C++ code of the Python bindings, with the/python/pattern in-ignore-filename-regex. The Python tests run this code through the installed wheel, and the step already passes the extension module tollvm-covas an-object./python/pattern is removed from-ignore-filename-regex. The patterns for the build folders, the tests, the virtual environment, the Python headers,mlx/3rdparty/andmlx/backend/cuda/do not change.mlx/andpython/srctogether. A new row countspython/srcalone. The row "All ofmlx/" and the per-directory rows do not change.reportfunction takes one or more directories, so one report can covermlx/andpython/src.mlx/and the Python bindings inpython/src.coverage-reportartifact (report.txt) is now the report for the Total row.No library code changes.
fork.yamlalready lists.github/workflows/fork-tests.yml, so nofork.yamlchange is needed.Results
Fork Tests run on head 76dd8d4: https://github.com/Layr-Labs/mlx/actions/runs/36744294779 (success). Before is #29's run 36587584091 on head 02720cc. Values are from the TOTAL rows in the step log and from the
coverage-reportartifact.mlx/python/srcThe rows for
mlx/backend/metal,mlx/backend/cpu,mlx/backend/common,mlx/distributedandmlx/iodo not change (within one line of the run on #29). Every file in the artifact is undermlx/orpython/src/, so removing the/python/pattern adds no other files.Job time: 24 min 33 s before, 26 min 41 s after. The report step takes 7 s in both runs. The difference is in the build (+1 min) and test (+1 min) steps, which this PR does not change.
Test plan
actionlint .github/workflows/fork-tests.yml: no findings./usr/bin/python3 scripts/check_forkdiff.py --upstream-ref refs/remotes/upstream/main: passes.CONTRIBUTING checklist
actionlintpasses on it.Dependencies
Merge this PR after #28 and #29. This branch is based on
ci/coverage-scope(#29's head, 02720cc), which is based on #28. The PR base isci/coverage-scope, so the diff shows only this change.Authorship
Written by Claude (Opus 5.5) under David Tai's direction.
🤖 Generated with Claude Code