Skip to content

test(aqua): allow commit metadata refs#10287

Merged
jdx merged 1 commit into
mainfrom
codex/aqua-sha-metadata
Jun 9, 2026
Merged

test(aqua): allow commit metadata refs#10287
jdx merged 1 commit into
mainfrom
codex/aqua-sha-metadata

Conversation

@jdx

@jdx jdx commented Jun 9, 2026

Copy link
Copy Markdown
Owner

Summary

  • allow baked aqua registry metadata to use either an upstream release tag or a pinned commit SHA
  • fixes unit tests after release-plz started vendoring aqua-registry from upstream main

Validation

  • cargo test --all-features aqua::standard_registry::tests::test_baked_registry_metadata

This PR was generated by an AI coding assistant.


Note

Low Risk
Test-only assertion change with no runtime or security impact.

Overview
Updates test_baked_registry_metadata so the baked aqua registry snapshot is valid when AQUA_STANDARD_REGISTRY_METADATA.tag is either a v… release tag or a 40-character hex commit SHA, not only release tags.

This keeps unit tests passing when the vendored registry is pinned to an upstream main commit (e.g. via release-plz) instead of a tagged release.

Reviewed by Cursor Bugbot for commit a15b7eb. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • Tests
    • Enhanced registry tag validation in tests to support additional acceptable formats.

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The pull request expands test assertions in test_baked_registry_metadata to accept registry tags in two formats: semantic version tags (starting with 'v') and 40-character hexadecimal strings (Git commit hashes).

Changes

Registry Tag Format Validation

Layer / File(s) Summary
Registry tag format validation in tests
src/aqua/standard_registry.rs
The test_baked_registry_metadata test assertion is broadened from checking only version-prefixed tags to also accepting 40-character ASCII hex strings, covering both semantic and commit hash tag formats.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

A test grows wiser, accepting more grace,
Version tags dance with hashes in place,
Forty hex digits, a commit's true face,
Registry tags now expand their embrace! 🐰✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: broadening test assertions to allow commit metadata references (commit SHAs) in addition to release tags.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/aqua-sha-metadata

Comment @coderabbitai help to get the list of available commands and usage tips.

@jdx jdx merged commit c594f7e into main Jun 9, 2026
22 of 23 checks passed
@jdx jdx deleted the codex/aqua-sha-metadata branch June 9, 2026 16:20
@greptile-apps

greptile-apps Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR relaxes a single test assertion in the baked aqua registry metadata test to accept either a semver-style release tag (starting with v) or a pinned 40-character hex commit SHA, accommodating a workflow change where release-plz now vendors the aqua-registry from upstream main using a commit SHA rather than a tagged release.

  • The test_baked_registry_metadata assertion is widened: it now passes if tag starts with 'v' or is a 40-hex-char full git SHA, matching both the old tag format and the new commit-SHA format produced by release-plz.
  • No production code is changed; the diff is entirely within the #[cfg(test)] module.

Confidence Score: 5/5

Safe to merge — the change is limited to a test assertion and correctly handles both tag formats.

Only one line of test code changes; production logic is untouched. The updated assertion correctly uses Rust's &&/|| precedence rules and accurately validates both a v-prefixed release tag and a full 40-character hex commit SHA.

No files require special attention.

Important Files Changed

Filename Overview
src/aqua/standard_registry.rs Test assertion updated to accept either a version tag (v-prefixed) or a 40-character hex commit SHA; logic is correct per Rust operator precedence, minor readability improvement possible with explicit parentheses.

Fix All in Claude Code

Reviews (1): Last reviewed commit: "test(aqua): allow commit metadata refs" | Re-trigger Greptile

Comment on lines +75 to +82
assert!(
AQUA_STANDARD_REGISTRY_METADATA.tag.starts_with('v')
|| AQUA_STANDARD_REGISTRY_METADATA.tag.len() == 40
&& AQUA_STANDARD_REGISTRY_METADATA
.tag
.chars()
.all(|c| c.is_ascii_hexdigit())
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The && and || operators sit at the same visual indent level, making the precedence ambiguous at a glance. In Rust && binds tighter than ||, so the logic is correct as written, but an explicit pair of parentheses would make the intent clear to anyone reading the test.

Suggested change
assert!(
AQUA_STANDARD_REGISTRY_METADATA.tag.starts_with('v')
|| AQUA_STANDARD_REGISTRY_METADATA.tag.len() == 40
&& AQUA_STANDARD_REGISTRY_METADATA
.tag
.chars()
.all(|c| c.is_ascii_hexdigit())
);
assert!(
AQUA_STANDARD_REGISTRY_METADATA.tag.starts_with('v')
|| (AQUA_STANDARD_REGISTRY_METADATA.tag.len() == 40
&& AQUA_STANDARD_REGISTRY_METADATA
.tag
.chars()
.all(|c| c.is_ascii_hexdigit()))
);

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code

@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

Hyperfine Performance

mise x -- echo

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.6.1 x -- echo 19.9 ± 1.5 16.9 25.3 1.00
mise x -- echo 21.3 ± 2.4 17.9 56.1 1.07 ± 0.14

mise env

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.6.1 env 20.4 ± 1.9 16.3 27.2 1.00
mise env 22.4 ± 1.6 18.1 28.9 1.10 ± 0.13

mise hook-env

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.6.1 hook-env 22.4 ± 3.2 17.9 32.7 1.11 ± 0.17
mise hook-env 20.1 ± 1.0 17.7 24.3 1.00
✅ Performance improvement for hook-env is 11%

mise ls

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.6.1 ls 15.2 ± 1.2 13.4 21.0 1.00
mise ls 17.4 ± 1.1 14.9 21.3 1.14 ± 0.11
⚠️ Inconclusive: ls measured 14% slower, but the relative uncertainty overlaps the 10% threshold.

xtasks/test/perf

Command mise-2026.6.1 mise Variance
install (cached) 148ms 151ms -1%
ls (cached) 63ms 64ms -1%
bin-paths (cached) 74ms 73ms +1%
task-ls (cached) 132ms 136ms -2%

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