alut: zero rest of buffer to match agave wire encoding #10052
Merged
ripatel-fd merged 1 commit intoJun 3, 2026
Conversation
intrigus-lgtm
requested review from
emwang-jump,
lidatong and
ripatel-fd
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a Makefile lint target and CI job to catch silent $(call ...) typos and ensure make-unit-test targets are registered to run by default, while fixing/annotating several existing Local.mk test registrations.
Changes:
- Introduce
make lint(and GitHub Actions workflow) to validate Local.mk macro calls and built-vs-run unit tests. - Fix several Local.mk issues: add missing
run-unit-testregistrations and annotate intentional exceptions. - Add zero-padding in
fd_alut_state_encodepayload buffer.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vinyl/Local.mk | Annotates a unit test as intentionally not auto-run (requires multi-tile setup). |
| src/util/wksp/Local.mk | Adds lint opt-out annotation for a problematic test and adjusts run registrations. |
| src/util/tpool/Local.mk | Registers test_tpool to run by default. |
| src/util/tmpl/Local.mk | Registers additional template/map/sort tests to run by default. |
| src/util/simd/Local.mk | Fixes a typo so a SIMD unit test is actually run. |
| src/util/shmem/Local.mk | Registers test_shmem to run by default. |
| src/util/sandbox/Local.mk | Registers test_sandbox to run by default when supported. |
| src/util/net/Local.mk | Annotates test_pcap as intentionally not auto-run (needs CLI input). |
| src/util/log/Local.mk | Registers test_log to run by default. |
| src/util/bits/Local.mk | Registers test_bits_tg to run by default. |
| src/tango/cnc/Local.mk | Annotates test_cnc as intentionally not auto-run (multi-tile requirement). |
| src/flamenco/runtime/fd_alut.h | Zero-pads remaining payload bytes after encoding. |
| src/flamenco/runtime/Local.mk | Registers additional flamenco unit tests to run by default. |
| src/flamenco/log_collector/Local.mk | Annotates a currently-broken test as intentionally not auto-run. |
| src/flamenco/accdb/Local.mk | Registers test_accdb_v0 to run by default. |
| src/discof/restore/Local.mk | Annotates a unit test as intentionally not auto-run (requires CLI arg). |
| src/discof/forest/Local.mk | Annotates a currently-broken test as intentionally not auto-run. |
| src/disco/keyguard/Local.mk | Registers test_keyload to run by default. |
| src/choreo/eqvoc/Local.mk | Annotates a currently-broken test as intentionally not auto-run. |
| src/choreo/Local.mk | Registers test_choreo_base to run by default. |
| src/ballet/sbpf/Local.mk | Annotates a unit test as intentionally not auto-run (requires CLI arg). |
| src/ballet/reedsol/Local.mk | Registers test_reedsol to run by default. |
| src/ballet/pb/Local.mk | Registers test_pb to run by default. |
| src/ballet/lthash/Local.mk | Registers test_lthash to run by default. |
| src/ballet/hmac/Local.mk | Fixes a typo so test_hmac is actually run. |
| src/ballet/bigint/Local.mk | Fixes a typo in the header-list macro name. |
| src/ballet/aes/Local.mk | Registers test_aes to run by default. |
| contrib/lint/check_makefiles.py | Adds the Local.mk linter (macro-name validation + built-but-not-run detection). |
| config/everything.mk | Adds lint target + help text and wires it into aux rules. |
| .github/workflows/on_pull_request.yml | Adds a lint job to PR CI (non-draft only). |
| .github/workflows/lint.yml | New reusable workflow that runs make lint. |
Comment on lines
+88
to
+90
| m = re.match(r"\s*([A-Za-z0-9_.+-]+)\s*[:!?]?=", line) | ||
| if m: | ||
| names.add(m.group(1)) |
| $(call make-unit-test,test_wksp_admin,test_wksp_admin,fd_util) | ||
| $(call make-unit-test,test_wksp_user,test_wksp_user,fd_util) | ||
| $(call make-unit-test,test_wksp_helper,test_wksp_helper,fd_util) | ||
| $(call make-unit-test,test_wksp_helper,test_wksp_helper,fd_util) # lint-no-run-unit-test, appears broken? |
ripatel-fd
requested changes
Jun 3, 2026
ripatel-fd
left a comment
ripatel-fd
left a comment
Contributor
There was a problem hiding this comment.
Could you split up the CI changes and the ALUT fix please?
For CI, a few of those tests are included in "script tests" instead, btw.
Fixes test_wire_encode_none_matches_agave FAIL: fd_memeq( actual, expected, sizeof(expected)
intrigus-lgtm
force-pushed
the
intrigus/fix/alut-test
branch
from
d1ddff1 to
631a076
Compare
Contributor
Author
|
Rebased. |
ripatel-fd
approved these changes
Jun 3, 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.
No description provided.