test: wire up the CT suites ELP reported as unreachable, and bump erlang-ci - #176
Merged
Conversation
…ang-ci
ELP's W0008 flagged three functions in the coverage suites as unreachable test
cases. Two are false positives and one is real, and they need opposite fixes.
`collect_batches/1` in both suites is a helper that drains batch messages, not
a test case - but a `name/1` in a CT suite that is not in all/0 is
indistinguishable from a test case ELP can see is never run. Taking the drain
window as a second argument says "helper" structurally rather than with a
suppression comment, and moves the timeout to the call site where it can be
read next to the batch size it is waiting on.
`insert_with_opts_returning_rows/1` was real: a test case that was never in
all/0 and, if it had been, asserted nothing - its whole body was `ok` under a
comment saying it meant to cover the on_conflict noop branch. That branch is
already covered by insert_with_opts_on_conflict_columns two functions above,
which inserts the same unique key twice under `{{columns, [...]}, nothing}`.
So the stub is dead weight rather than missing coverage, and it goes.
Pin moves to erlang-ci v2.1.7. That upgrades ELP from 2026-02-27 to 2026-08-10
(erlang-ci#71/#72/#73), which is the version that reports W0008 at all - the
bump and this cleanup have to land together or CI goes red on the three above.
v2.1.7 also compiles the test profile before linting, which is what stops ELP
reporting `proper_transformer:parse_transform/2` on the two suites that
include proper.hrl.
Verified: kura_coverage_SUITE + kura_coverage2_SUITE 87/87, elp lint 0 errors
(was 5), fmt and xref clean. kura_resilience_tests fails locally on
docker-compose control because another checkout holds port 5555; it fails
identically on unmodified main and is skipped in CI.
🟢 Code Coverage — 92.4%2612 of 2826 lines covered. 🟡 ELP Lint — 29 warnings34 diagnostics found. See job logs for details. |
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
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.
Found while auditing whether kura's ELP settings matched reality after widgrensit/asobi#436.
kura's
enable-elp-lint: truehas been green, but only because the pinned erlang-ci (v2.0.5) installs ELP2026-02-27, which does not carry theW0008diagnostic. Under current ELP the tree has 5 lint errors, so a routine pin bump would have turned CI red with no obvious connection to the bump.Three findings, two different fixes
collect_batches/1in both coverage suites is a helper that drains batch messages, not a test case. ELP cannot tell: aname/1in a CT suite that is not inall/0looks exactly like a test case that never runs. Rather than a suppression comment, it now takes the drain window as a second argument - that says "helper" structurally, and puts the timeout at the call site next to the batch size it is waiting on.insert_with_opts_returning_rows/1was the real one, and worse than ELP could tell. It was never inall/0, and had it been, it asserted nothing:The branch it names is already covered by
insert_with_opts_on_conflict_columnstwo functions above, which inserts the same unique key twice under{{columns, [name, email]}, nothing}. So this is dead weight, not missing coverage, and it is removed rather than wired in - a test that runs and asserts nothing is worse than no test.Pin bump has to land with it
v2.0.5 -> v2.1.7. That moves ELP to
2026-08-10(Taure/erlang-ci#71, #72, #73), which is the version that reportsW0008at all - so the bump and this cleanup have to go together or CI goes red on the three above.v2.1.7 also compiles the test profile before linting, which is what removes the other two errors:
kura_custom_type_testsandkura_types_prop_testsboth-include_lib("proper/include/proper.hrl"), and under the oldrebar3 compilethose deps were never fetched, so ELP reportedundefined function proper_transformer:parse_transform/2for both. Measured on this tree: 5 errors before, 3 after the profile change, 0 after this cleanup.Verification
kura_coverage_SUITE+kura_coverage2_SUITE: 87/87.elp lint: 0 errors (29 warnings, unchanged), was 5.rebar3 fmt --check,xref: clean.kura_resilience_testsfails locally (2 failures, 1 cancelled) because it takes docker-compose control and another checkout holds port 5555. It fails identically on unmodifiedmainand is skipped in CI, so it is environmental and unrelated.Not in this PR
enable-elp-eqwalizestaysfalse: 92 errors on this tree (76incompatible_types, 16redundant_nowarn_function). Worth noting the 16redundant_nowarn_functionare cheap - eqwalizer saying suppressions are no longer needed - so the real work is closer to 76. asobi tracks the equivalent in widgrensit/asobi#435 if you want the same shape of plan here.