Skip to content

Skip standalone test-fixture generation when embedded#32

Merged
swilly22 merged 2 commits into
mainfrom
embed-ldap-config-prefix
Jun 28, 2026
Merged

Skip standalone test-fixture generation when embedded#32
swilly22 merged 2 commits into
mainfrom
embed-ldap-config-prefix

Conversation

@dudizimber

@dudizimber dudizimber commented Jun 25, 2026

Copy link
Copy Markdown

build.rs runs scripts/generate_test_certificates.sh in debug builds to set up TLS certs for the crate's own integration tests. That script walks up the tree looking for a directory literally named 'valkey-ldap' and loops forever when the crate is vendored as a submodule under a different name (e.g. modules/ldap), hanging the build. The fixtures are never used when the crate is embedded into a host module, so skip them whenever the 'embed' feature is enabled.

Summary by CodeRabbit

  • Bug Fixes
    • Updated debug build behavior so fixture generation and copying no longer run when the app is built with embedding enabled.
    • This helps avoid unnecessary build-time file creation and makes embedded builds behave more consistently.

build.rs runs scripts/generate_test_certificates.sh in debug builds to set up
TLS certs for the crate's own integration tests. That script walks up the tree
looking for a directory literally named 'valkey-ldap' and loops forever when the
crate is vendored as a submodule under a different name (e.g. modules/ldap),
hanging the build. The fixtures are never used when the crate is embedded into a
host module, so skip them whenever the 'embed' feature is enabled.
@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b15697e1-b61a-4dd6-bce9-0af6a7bb56e3

📥 Commits

Reviewing files that changed from the base of the PR and between 2f9e86a and 1d3009a.

📒 Files selected for processing (1)
  • build.rs

📝 Walkthrough

Walkthrough

The build script now skips debug-only fixture generation and copying when the crate is built with the embed feature. It derives an embedded flag from CARGO_FEATURE_EMBED and uses it in the debug guard.

Changes

Embed-aware build script gating

Layer / File(s) Summary
Debug fixture gate
build.rs
build.rs now derives an embedded flag from CARGO_FEATURE_EMBED and skips debug-only certificate generation and fixture copying when embedding is enabled; inline comments describe the behavior.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

🐰 I hopped through the build-script gate,
With embed on, I wait and wait.
Debug carrots stay tucked away,
While fixtures skip their little play.
Thump-thump—quiet builds today!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: skipping standalone test-fixture generation when the crate is embedded.
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.

✏️ 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 embed-ldap-config-prefix

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@swilly22
swilly22 merged commit aec7040 into main Jun 28, 2026
8 checks passed
@swilly22
swilly22 deleted the embed-ldap-config-prefix branch June 28, 2026 09:10
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.

2 participants