Skip to content

test: cover the remaining unexercised expected members - #80

Open
steve-downey wants to merge 1 commit into
bemanproject:mainfrom
steve-downey:coverage-fill-uncovered-members
Open

test: cover the remaining unexercised expected members#80
steve-downey wants to merge 1 commit into
bemanproject:mainfrom
steve-downey:coverage-fill-uncovered-members

Conversation

@steve-downey

Copy link
Copy Markdown
Member

Coverage review of make TOOLCHAIN=gcc-16 coverage turned up 24 out-of-line definitions that no test instantiated, plus four that were instantiated but never called.

Nearly all of them were the same shape: a const& overload shadowed by its && twin. Several existing tests were misnamed accordingly:

  • expected<void>: construct from unexpected const& passed an rvalue and bound the && overload
  • expected<void,E&>: convert from expected<void, G&> used identical types and so invoked the copy constructor
  • the const monadic tests all wrote const expected<...> e; std::move(e).or_else(...), hitting const&& and leaving every const& monadic overload untouched

Tests added

  • expected<void, E> — converting move ctor, const unexpected& ctor/assign, error_or() &&, value() && on a valued expected, and the const& overloads of or_else / transform / transform_error
  • expected<T&, E> — converting move ctor, const unexpected& ctor/assign, unexpect_t + initializer_list ctor, value() &&, all four ref-qualified error() overloads, error_or() &&, or_else const&
  • expected<T&, E&> — converting copy/move ctors (Derived& -> Base&, int& -> const int&)
  • reference-Econst unexpected<G&> lvalue ctor/assign across all three specializations
  • expected<T, E>::value() const&& on a valued expected
  • bad_expected_access<void>::what() — reachable only from a derived class: its special members are protected and every bad_expected_access<E> overrides what(), so dispatch through a base reference never lands on it

Two things worth a look in review

The initializer_list test passes its trailing argument as a runtime lvalue on purpose. With every argument a constant expression the whole initializer is constant-evaluated — both that constructor and init_list_type's are constexpr — and GCC folds it at compile time even under the Gcov profile's -O0 -fno-inline, emitting no symbol and no coverage record. Confirmed with nm on the object file. The constant-evaluated path is asserted separately with a static_assert, so both paths are covered deliberately rather than by accident.

BEMAN_EXPECTED_TRAP() lines are excluded from the gcovr report. The macro expands to __builtin_trap(); reaching one kills the process with SIGILL before libgcov flushes, so no .gcda is written at all — a death test would not merely fail to mark the line covered, it would discard the whole run's counters. Those lines are uncoverable by construction rather than untested, so the exclusion lives in cmake/gcovr.cfg.in rather than as GCOVR_EXCL_LINE markers in a header written for standardization. The guarding if (has_val_) is deliberately left in, so branch coverage still reports that no test drives the precondition check.

Numbers

before after
instrumented lines 97.0% (686/707) 100% (798/798)
functions 95.2% (853/896) 95.7% (915/956)

The remaining uncovered functions are all redundant template instantiations (e.g. bad_expected_access<traced>::what()) of source lines other instantiations already exercise, forced by trait probes rather than by calls. No distinct API surface is left untested.

1136/1136 tests pass; clang-format clean.

Incidental finding, not addressed here

There are 21 trap call sites in the header but only 15 ever appear in coverage data — the expected<T&, E> traps are absent because expected_hardened.test.cpp only instantiates expected<T,E> and expected<void,E>, so the reference specializations are never compiled under -DBEMAN_EXPECTED_HARDENED at all. The exclusion pattern covers all 21 regardless, so it stays correct if that file grows. Whether the hardened build should instantiate the reference specializations is a separate question.

🤖 Generated with Claude Code

Coverage review of `make TOOLCHAIN=gcc-16 coverage` turned up 24 out-of-line
definitions that no test instantiated, plus four that were instantiated but
never called. Nearly all of them were the same shape: a `const&` overload
shadowed by its `&&` twin. Several existing tests were misnamed accordingly --
"construct from unexpected const&" passed an rvalue and bound the `&&`
overload; "convert from expected<void, G&>" used identical types and so
invoked the copy constructor; and the `const` monadic tests all wrote
`const expected<...> e; std::move(e).or_else(...)`, hitting `const&&` and
leaving every `const&` monadic overload untouched.

Adds tests for:

  - expected<void, E>: converting move ctor, const unexpected& ctor/assign,
    error_or() &&, value() && on a valued expected, and the const& overloads
    of or_else / transform / transform_error
  - expected<T&, E>: converting move ctor, const unexpected& ctor/assign,
    unexpect_t + initializer_list ctor, value() &&, all four ref-qualified
    error() overloads, error_or() &&, or_else const&
  - expected<T&, E&>: converting copy/move ctors (Derived& -> Base&,
    int& -> const int&)
  - reference-E const unexpected<G&> lvalue ctor/assign across all three
    specializations
  - expected<T, E>::value() const&& on a valued expected
  - bad_expected_access<void>::what(), reachable only from a derived class:
    its special members are protected and every bad_expected_access<E>
    overrides what(), so dispatch through a base reference never lands on it

The initializer_list test passes its trailing argument as a runtime lvalue on
purpose. With every argument a constant expression the whole initializer is
constant-evaluated -- both that constructor and init_list_type's are constexpr
-- and GCC folds it at compile time even under the Gcov profile's -O0
-fno-inline, emitting no symbol and no coverage record. The constant-evaluated
path is asserted separately with a static_assert.

Also excludes BEMAN_EXPECTED_TRAP() lines from the gcovr report. The macro
expands to __builtin_trap(); reaching one kills the process with SIGILL before
libgcov flushes, so no .gcda is written at all -- a death test would not merely
fail to mark the line covered, it would discard the whole run's counters. Those
lines are uncoverable by construction rather than untested. The guarding
`if (has_val_)` is deliberately left in, so branch coverage still reports that
no test drives the precondition check.

Instrumented-line coverage goes from 97.0% (686/707) to 100% (798/798);
1136 tests pass.
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