Skip to content

Test what main's coverage misses, and exclude what no input reaches - #656

Open
eriknw wants to merge 18 commits into
16-asv-benchmarksfrom
51-coverage-main-gaps
Open

eriknw wants to merge 18 commits into
16-asv-benchmarksfrom
51-coverage-main-gaps

Conversation

@eriknw

@eriknw eriknw commented Oct 28, 2026 •

Copy link
Copy Markdown
Member

Stacked on #596, below #651. Tests for lines on main that Codecov showed as never run, and
# pragma: no cover (or no branch) for the ones no input can reach. The library changes
are pragmas and comments only, plus one stale "BRANCH NOT COVERED" comment removed: no
behavior changes here. The bugs these tests turned up are fixed in their own PRs lower in
the stack, or queued (below).

What the new tests cover:

  • Monoids and aggregators (test_op.py, test_pickle.py): a bool field's min and max
    identities; a date or text field, which has none, named in the monoid's KeyError, and
    the aggregator not working with the UDT; a user monoid without an identity; a named
    monoid unpickled after its name is gone, registered again with its identity and
    is_idempotent; agg.sum[udt].jit_c_name following its monoid.
  • IndexBinaryOp (test_indexbinary.py, test_ssjit.py): no theta binds 0, through the
    typed op or the op given a dtype; C and GraphBLAS scalar thetas; a UDT-typed op given a
    value that is not one; an op with no theta bound refused as an operator by reduce; the
    errors before SuiteSparse:GraphBLAS 9.4; gb.indexbinary.ss absent on the vanilla
    backend.
  • The JIT compiler repair (test_ssjit.py, against a stand-in config):
    fix_jit_config(probe=False), which enables the JIT without a test compile; no
    CONDA_PREFIX, so the compiler, flags and libraries come from sysconfig, or None when
    Python records no compiler; a baked-in -isysroot that does not exist, with and without
    xcrun; the import-time repair on a SuiteSparse with no JIT options; an error the JIT
    control does not explain re-raised as it came.
  • SuiteSparse extras: dtypes.ss.register_new refusing a bad name or typedef before
    any compile; the element count check of array settings in gb.ss.config (replacing a
    commented-out check on memory_pool, which is no longer an option);
    Matrix.ss.serialize of a UDT whose name is too long for the blob; deserializing a blob
    whose integer sizes SuiteSparse cannot read; gb.ss.diag of a Vector without options;
    the shape of a full array-UDT import without nrows and ncols; test_ss_pack_hyperhash
    with 1025 rows, so SuiteSparse computes the hash on every version checked (8.2.1, 10.5.0
    and 10.5.1), where with 2 rows the pack arm ran on some CI draws only.
  • Elsewhere: Matrix Market I/O through a stand-in fast_matrix_market (CI does not
    install it, so that engine never ran); an infix ewise_add of two infix expressions;
    unary.identity and unary.one on a UDT without Numba; the text and HTML reprs of an apply
    naming its UDT scalar operand.

Each pragma carries a reason word, (safety) for unreachable by construction,
(defensive), (unused), (only on failure) or (child process), and a one-line comment
where the reason is not already written beside it. In the library: a UDT's C typedef
asked for without its numpy type, the explicit layout of a dtype that is neither a record
nor an array, get_typed_op's except arm, the typed op finalizer's shutdown guards,
IndexBinaryOp guards that its builder already enforces, _scalar_identity with a name
other than min or max, the int64 config arms (no option has that type since memory_pool
went), a deserialize status check that the read before it already made, and lookups that
every supported build satisfies. In the tests: code that runs only in a spawned child
process (the cross-process pickle tests), on failure, or on a re-run in the same process.
Review tried to reach each excluded line; the four it could reach are tested or counted
again instead.

Bugs found, all on main too:

Queued, not fixed in this stack:

The same survey found why most of test_ssjit.py's JIT tests skip in CI. The compiler is
fine on Linux and macOS; psg's wheels build GraphBLAS without the JIT, and on Windows the
conda repair did not look where conda installs compilers. #636 now runs test_ssjit.py with
--runslow in the Linux and macOS notebooks jobs (and installs the psg version a wheel or
source draw picks, which had always been the newest), and #652 finds a
conda compiler on Windows.

Coverage, measured locally (suitesparse backend, Numba, --runslow, the full suite) before
review added three more tests: missed lines went from 926 to 836 and partial branches from
546 to 495, 95.90% to 96.32%. Most of what is left is covered only on other CI draws (older
SuiteSparse, vanilla, no Numba) or is #591's own code.

Every commit that adds tests was checked by breaking the code under test: 24 of 27 such
changes made the new test fail, and the other three changed nothing observable; the three
tests added in review each fail with their code broken. The touched
test files pass in random order with fixed seeds in eight configurations (including no
Numba on both backends, CI's C-scalar pass, numpy 2.2, SuiteSparse:GraphBLAS 8.2 and Python
3.15). The full suite passes with --runslow, without Numba on both backends, and in the
C-scalar pass; on Python 3.15, numpy 2.2 and SuiteSparse:GraphBLAS 8.2 the only failure is
test_version, from the local environments.

A CI job that always compiles the JIT (added in review, at Erik's request). The JIT tests
used to run only on random draws that got the suitesparse backend, --runslow, Numba and a
GraphBLAS that compiles, so Codecov's total moved by up to 0.7 points between runs.
Add a CI job that always compiles the JIT, so its coverage stops swinging adds one fixed
entry, (ubuntu-latest, pytest_jit): Python 3.15 and the newest psg from conda-forge, Numba
always, and the full suite with --runslow. Fail the JIT tests instead of skipping them under
GRAPHBLAS_REQUIRE_JIT
makes that job turn red when it cannot compile, where it would otherwise
skip the JIT tests and lose their coverage. Every other job skips as before.

@codecov

codecov Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.56631% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 99.33%. Comparing base (641049b) to head (bc87dda).

Additional details and impacted files
@@                  Coverage Diff                  @@
##           16-asv-benchmarks     #656      +/-   ##
=====================================================
+ Coverage              92.45%   99.33%   +6.88%     
=====================================================
  Files                    109      109              
  Lines                  34884    35086     +202     
  Branches                4001     3990      -11     
=====================================================
+ Hits                   32251    34854    +2603     
+ Misses                  2408      144    -2264     
+ Partials                 225       88     -137     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@eriknw
eriknw force-pushed the 51-coverage-main-gaps branch from 2a430a5 to cc828b8 Compare October 28, 2026 13:21
@eriknw
eriknw force-pushed the 51-coverage-main-gaps branch from cc828b8 to 71999d8 Compare October 29, 2026 02:01
@eriknw
eriknw force-pushed the 51-coverage-main-gaps branch 3 times, most recently from 8d8fa26 to b2cb5df Compare November 3, 2026 16:44
@eriknw
eriknw removed this pull request from stack #657 November 3, 2026 16:44
@eriknw
eriknw added this pull request to stack #660 November 3, 2026 16:45
@eriknw
eriknw force-pushed the 51-coverage-main-gaps branch 2 times, most recently from a8569d0 to fcfce9c Compare November 3, 2026 23:09
@eriknw
eriknw removed this pull request from stack #660 November 3, 2026 23:09
@eriknw
eriknw added this pull request to stack #662 November 3, 2026 23:09
@eriknw
eriknw removed this pull request from stack #662 November 3, 2026 23:11
@eriknw
eriknw added this pull request to stack #664 November 3, 2026 23:12
@eriknw
eriknw force-pushed the 51-coverage-main-gaps branch from fcfce9c to dcb0e67 Compare November 4, 2026 18:59
@eriknw
eriknw removed this pull request from stack #664 November 4, 2026 20:50
@eriknw
eriknw force-pushed the 51-coverage-main-gaps branch from dcb0e67 to 2740545 Compare November 4, 2026 20:50
@eriknw
eriknw added this pull request to stack #666 November 4, 2026 20:51
@eriknw
eriknw force-pushed the 51-coverage-main-gaps branch 3 times, most recently from e49e788 to 64b656f Compare November 9, 2026 15:46
@eriknw
eriknw added this pull request to stack #670 November 12, 2026 06:17
@eriknw
eriknw force-pushed the 51-coverage-main-gaps branch 10 times, most recently from 51912b3 to cee7fe2 Compare November 13, 2026 04:37
…kling

A bool field takes min's and max's identity as logical and/or; a date or
text field has none, so the monoid names the field and the aggregator does
not work with the UDT; a user monoid without an identity says so. A named
monoid unpickled where its name is gone is registered again with its
identity and is_idempotent. agg.sum[udt].jit_c_name follows its monoid.

The name check in _scalar_identity and the initializer's lookup, which no
input reaches, get pragmas with the reason.
…rors

Binding no theta binds 0, through the typed op or the op given a dtype; a
C scalar or GraphBLAS scalar theta binds through either; a UDT-typed op
given a value that is not one raises that value's TypeError. The typed op
reports its parent's function. An op with no theta bound, UDT or not, is
refused as an operator by reduce. Before SuiteSparse:GraphBLAS 9.4 both
register_anonymous and indexbinary.ss.register_new say what is missing,
and gb.indexbinary.ss is absent on the vanilla backend.

The arms no IndexBinaryOp can reach, and a test's re-run cleanup, get
pragmas with the reason.
… probing

fix_jit_config(probe=False) enables the JIT without trying a compile.
Without CONDA_PREFIX the compiler, flags and libraries come from sysconfig,
and a Python that records no compiler gets None and an untouched config.
A baked-in -isysroot that does not exist takes xcrun's SDK, or is dropped
when there is no xcrun. The import-time repair leaves a SuiteSparse with no
JIT options alone, and an error the control does not explain is re-raised
as it came. All run against a stand-in config.

The no-environment test drops CONDA_PREFIX with monkeypatch rather than by
hand, so it has no branch that depends on the environment.
A name that is not an identifier, a typedef that does not declare the name,
and a typedef of something other than a struct are refused with ValueError
before SuiteSparse is asked to compile anything, and register nothing.
Setting bitmap_switch to the wrong number of values raises ValueError and
leaves the setting as it was. This replaces the commented-out check on
memory_pool, which is no longer an option. No option has an int64_t type
since then, so those get and set arms get pragmas.
As test_vector.py's test_udt does for Vector: an anonymous UDT with a long
numpy repr round-trips given dtype=, and a named one is found by its
registered name.
CI does not install fast_matrix_market, so the engine was never used. A
stand-in module that reads and writes with scipy plays it: engine "auto"
and "fmm" both use it, each with its deprecation warning, and a version
before 1.7 is given a scipy sparse matrix to write rather than an array.
Each gets a pragma with its reason, and a comment where the reason is not
already written next to it:
- dtypes: a UDT's C typedef is asked for only with its numpy type, and the
  explicit layout dict recurses only into records and arrays;
- get_typed_op: two built-in types always unify, and a UDT beside another
  type returns before that;
- typed op finalizer: the shutdown and no-free-function guards;
- the GxB_EXHAUSTED check and the lookup of unary.abs and unary.ainv, which
  every supported build has.
Both operands of the outer | are themselves expressions: with the same
dtype the inner typed op is reused, and with another dtype the result is
promoted.
test_diag always passed nthreads, so the arm without options never ran.
The cross-process pickle tests unpickle and compute in a spawned child,
which coverage does not measure; the pre-test cleanups run only when an
earlier run in the same process left the name behind; the identity
mismatch messages are built only on failure; and a stand-in lookup passes
other keys through only if the code under test asks for them. The JIT
parity check's comparison is one expression, so it has no failure-only
line.
test_ss_pack_hyperhash packed the hash back only when SuiteSparse had
computed one, which for its two rows SuiteSparse 8.2 does and 10.5 does
not, so the pack arm ran on some CI draws only. With 1025 rows every
version checked computes it (8.2.1, 10.5.0 and 10.5.1); the hash stays
optional, so the guard stays, with a pragma.
Without Numba, applying unary.identity or unary.one to a UDT raises the
usual KeyError that the op does not work with it.
SuiteSparse 10 refuses a blob whose encoding word asks for integer sizes
larger than it knows, as a later version may write, when GrB_NAME's size
is read from it, though it read the dtype name. A UDT Matrix blob edited
so raises InvalidObject naming the read that failed (on SuiteSparse 9 the
edit makes the type code invalid, and the dtype name read fails first).

Reading GrB_NAME itself checks the blob as reading its size just did, so
that status check gets a pragma.
The values of a full array-UDT Matrix carry its shape in their first two
of three dimensions, and its export then leaves out nrows and ncols; an
iso export has a single element, so importing it without nrows and ncols
asks for them. That arm of get_shape is covered now, so its "BRANCH NOT
COVERED" marker goes.
apply passes a UDT C scalar to GraphBLAS through a pointer object, and
both the text and the HTML repr of the expression name the scalar, which
the HTML one asks the pointer for.
The JIT tests skip where SuiteSparse cannot compile: no JIT in the build,
no usable C compiler, a failed probe. That is right on a random CI draw,
but it also lets a setup that should compile drop every JIT test, and
their coverage, without a red job. With GRAPHBLAS_REQUIRE_JIT set, those
skips (now one helper, skip_without_jit, in conftest.py) fail instead,
in test_ssjit.py, test_op.py's udt_op_path and test_dtype.py.

A GraphBLAS 7 or a missing Numba skips whole tests by marker, and a
_require_jit_on that sees jit_c_control "on" runs its test even when the
probe failed, so test_required_jit_works checks all three where the
variable is set, and skips elsewhere.
…ging

The JIT tests ran, and counted for coverage, only on the random draws that
happened to get the suitesparse backend, --runslow, Numba and a GraphBLAS
that compiles, so Codecov's total moved by up to 0.7 points from run to
run. Add one fixed entry, (ubuntu-latest, pytest_jit), outside the draw:
Python 3.15 from conda-forge with the newest psg, Numba always (no random
skip) and conda-forge's C compiler, running the suite once with --runslow
and --backend=suitesparse, and uploading coverage like the others. That
setup compiled in recent runs (graphblas 10.5.2, psg 10.5.2.0, numba
0.68.1: test_ssjit passed but for the IndexBinaryOp version check).

The job sets GRAPHBLAS_REQUIRE_JIT, so a setup that stops compiling fails
it instead of skipping the JIT tests. It skips the C scalar pass, which
the other jobs run. Codecov counts no uploads (.codecov.yml sets no
after_n_builds), so the twelfth merges like the other eleven.
@eriknw
eriknw force-pushed the 51-coverage-main-gaps branch from cee7fe2 to bc87dda Compare November 13, 2026 04:40
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