build: derive TEST_BINS from the build rules, not the .c files (corrects #731) - #733
Merged
Conversation
Corrects #731, which globbed tests/test_*.c. That was wrong in the opposite direction from the hand-written list it replaced: it promoted files that deliberately have no build rule into gates. On dev the four such files were known and excluded by name, so the set came out identical and the mistake was invisible; on a contributor branch it is not. Merging dev into #529 promoted that branch's tests/test_fp8_e2e_loader.c -- a file monotophic has but never gated -- and the build failed on an undefined reference to sqrtf. Having a build rule is the honest definition of a gate, so derive the list from the rules. TEST_EXCLUDE drops from five entries to one: test_uring, which has a rule but is Linux-only and is appended conditionally just below. The property that motivated all of this is unchanged and now stronger: adding a gate means adding your .c and its own rule, which land in different places in the file. There is no shared list left to conflict on at all. Verified identical to current dev (31 entries, same names) and make test-c green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
JustVugg
added a commit
to monotophic/colibri
that referenced
this pull request
Jul 31, 2026
Resolved by a maintainer instead of asking for another rebase. The only conflicting hunk was the hand-written TEST_BINS line, which dev no longer has: gates are derived from the build rules, so this branch's tests are picked up by their own rules and the manual list entry is dropped. No other file conflicted and no commit on this branch was rewritten. Verified before pushing: every test this branch adds a rule for is in the gate set, and make test-c passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
JustVugg
added a commit
to terrizoaguimor/colibri
that referenced
this pull request
Jul 31, 2026
Resolved by a maintainer instead of asking for another rebase. The only conflicting hunk was the hand-written TEST_BINS line, which dev no longer has: gates are derived from the build rules, so this branch's tests are picked up by their own rules and the manual list entry is dropped. No other file conflicted and no commit on this branch was rewritten. Verified before pushing: every test this branch adds a rule for is in the gate set, and make test-c passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
JustVugg
added a commit
to monotophic/colibri
that referenced
this pull request
Jul 31, 2026
Resolved by a maintainer instead of asking for another rebase. The only conflicting hunk was the hand-written TEST_BINS line, which dev no longer has: gates are derived from the build rules, so this branch's tests are picked up by their own rules and the manual list entry is dropped. No other file conflicted and no commit on this branch was rewritten. Verified before pushing: every test this branch adds a rule for is in the gate set, and make test-c passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
JustVugg
added a commit
to terrizoaguimor/colibri
that referenced
this pull request
Jul 31, 2026
Resolved by a maintainer instead of asking for another rebase. The only conflicting hunk was the hand-written TEST_BINS line, which dev no longer has: gates are derived from the build rules, so this branch's tests are picked up by their own rules and the manual list entry is dropped. No other file conflicted and no commit on this branch was rewritten. Verified before pushing: every test this branch adds a rule for is in the gate set, and make test-c passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
JustVugg
added a commit
to jeswr/colibri
that referenced
this pull request
Jul 31, 2026
…l.h in the colibri rule Two conflicting hunks, resolved by a maintainer rather than asking for a rebase. TEST_BINS: dev no longer has a hand-written list, so this branch's entry is dropped and its test is picked up by its own rule. The colibri$(EXE) rule: both sides were right. This branch adds abl.h to the prerequisites; dev added $(VK_OBJ)/$(VK_SPV) and the Vulkan backend rules. Merged rather than picked -- dev's line with abl.h reinstated -- so the ablation harness still rebuilds when its header changes and the Vulkan build keeps working. make test-c passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
5 tasks
JustVugg
added a commit
to monotophic/colibri
that referenced
this pull request
Jul 31, 2026
Resolved by a maintainer instead of asking for another rebase. The only conflicting hunk was the hand-written TEST_BINS line, which dev no longer has: gates are derived from the build rules, so this branch's tests are picked up by their own rules and the manual list entry is dropped. No other file conflicted and no commit on this branch was rewritten. Verified before pushing: every test this branch adds a rule for is in the gate set, and make test-c passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
JustVugg
added a commit
to jeswr/colibri
that referenced
this pull request
Jul 31, 2026
Resolved by a maintainer instead of asking for another rebase. The only conflicting hunk was the hand-written TEST_BINS line, which dev no longer has: gates are derived from the build rules, so this branch's tests are picked up by their own rules and the manual list entry is dropped. No other file conflicted and no commit on this branch was rewritten. Verified before pushing: every test this branch adds a rule for is in the gate set, and make test-c passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 1, 2026
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.
Correction to #731, found by merging
devinto a real contributor branch rather than by reasoning about it.What #731 got wrong
#731 replaced the hand-written
TEST_BINSline with a glob oftests/test_*.c. That was wrong in the opposite direction from the list it replaced: it promotes anytest_*.cfile into a gate, including ones that deliberately have no build rule.On
devthe four such files were known and excluded by name, so the resulting set was identical and the mistake was invisible. On a contributor branch it is not. Mergingdevinto #529 promoted that branch'stests/test_fp8_e2e_loader.c— a file @monotophic has but never gated — and the build failed:Every future branch carrying an un-wired test would have hit the same thing at merge time.
The fix
Having a build rule is the honest definition of a gate, so derive the list from the rules instead of from the files.
TEST_EXCLUDEdrops from five entries to one —test_uring, which has a rule but is Linux-only and is appended conditionally just below.The property that motivated all of this is unchanged and now stronger: adding a gate means adding your
.cand its own rule, which land in different places in the file. There is no shared list left to conflict on at all.Verification
dev: 31 entries, same names.make test-cgreen.test_fp8_load,test_fp8_passthroughandtest_qt_addrowall included (they have rules), andtest_fp8_e2e_loadercorrectly left out.One file, build system only.