Add Azurite test support for AzureBlobStore - #2684
Muktarsadiq wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Thank you for the PR. We will check it out soon! |
d35c7e3 to
419234e
Compare
palfrey
left a comment
There was a problem hiding this comment.
Thanks for doing this. There's a few issues though.
419234e to
6a12eca
Compare
This comment has been minimized.
This comment has been minimized.
6a12eca to
5ee429d
Compare
|
I would suggest running all of this locally under Bazel with nix, so you can fix the variety of lint failures there as well as run |
5ee429d to
aec4854
Compare
aec4854 to
c319eeb
Compare
|
Ran bazel-retry test //nativelink-store:integration under Nix. 25/27 pass. The 2 failures were azurite_store_test_test fails exactly as flagged in my earlier comment (sandbox has no network to run bun install), and grpc_store_test_test failed on an unrelated pre existing test with a hardcoded 5s timeout, which I believe was resource pressure flakiness on my VM rather than a real regression |
9332514 to
c319eeb
Compare
|
@Muktarsadiq You are very close. Let me know if you need any help. The answer is in the logs here. |
c319eeb to
7f9e954
Compare
7f9e954 to
a782114
Compare
Adds an embedded Azurite (Azure Storage emulator) test runner, mirroring the existing mongo_runner pattern, and closes TraceMachina#2511. Azurite has no standalone binary distribution, so the runner invokes a locally npm-installed azurite-blob directly rather than downloading one. The store is pointed at it via ExperimentalAzureSpec.sas_url rather than endpoint, since endpoint alone routes through WorkloadIdentityCredential, which Azurite cannot satisfy. A SAS signer generates both a container scoped Service SAS for test operations and an Account SAS for one time container bootstrapping, since a container SAS cannot authorize creating the container it names. Also fixes a process crash discovered while building this: Azurite logs every request to stdout, and reading that pipe only long enough to capture the startup port left it undrained afterward, causing the next write to block and crash the whole process. stdout is now drained for the process's full lifetime instead. CI: adds actions/setup-node and npm ci to native-cargo.yaml so azurite-blob is available on both OS legs before cargo test runs.
a782114 to
d4dcb05
Compare
What and why
Adds Azurite (Azure Storage emulator) support to the test suite for
AzureBlobStore, so it's exercised against a real emulator rather thanonly mocks. Follows the existing
mongo_runnerembedded-runner pattern.Fixes #2511
How was this verified?
Ran
cargo test -p nativelink-store --test azurite_store_testlocally:upload_and_get_data,upload_empty_data, andzero_len_items_exist_checkall pass against a realAzureBlobStoretalking to a locally spawned Azurite instance no mocking. Two
#[ignore]d tests document manual verification of the SAS signer andcontainer-bootstrap logic, done independently before wiring them into
the automatic flow.
Also ran the full suite under Bazel/Nix per review feedback
(
bazel-retry test //nativelink-store:integration ...): 25/27 pass.The 2 known failures are
azurite_store_test_test(Bazel's sandboxedtest execution has no network access to run
bun install, soazurite-blobisn't present at runtime, same root cause as thep.mongodbsymlink hackmongo_runnerneeds inflake.nix) and aone-off
grpc_store_test_testtimeout that passes cleanly inisolation, confirmed as resource pressure flakiness rather than a
real regression.
Fixed 11 Clippy findings surfaced by the Bazel/Nix run (import
ordering,
tokio::spawn→nativelink_util::background_spawn!, doccomment formatting,
#[tokio::test]→#[nativelink_test], etc.),and a Windows-specific bug where the runner looked for
azurite-blobwithout the
.cmdextension npm/bun generate on that platform.Could not verify
bazel test //...(full monorepo) or the Nix-drivenpre-commit hooks directly at first, due to macOS 12/Monterey being
unsupported by the current Nix installer. Later verified both via an
Ubuntu VM once set up.
Risk
Low. This is purely additive test infrastructure, no production code
(
azure_blob_store.rsitself) is touched beyond adapting one call siteto match its
async/non-asyncsignature after an unrelated upstreamRust 1.97.1 toolchain bump changed it mid-review. New dev-only
dependencies (
chrono,hmac) and a Bun-based install step affecttest builds and CI only, not the shipped
nativelinkbinary. The knowngap is that the new Azurite tests don't yet execute under Bazel/Nix's
sandboxed test paths (see above), a coverage gap for this new suite,
not a regression, since the existing mocked
azure_blob_store_test.rsstill runs everywhere unaffected.
AI assistance
Used GitHub Copilot to help diagnose several CI failure logs during
review (Clippy findings, a stale commit false positive on an
AzureBlobStore::new()signature question, and the PR-template checkitself), verified each suggestion against the actual source/log
output before applying, and caught at least one incorrect diagnosis
from it in the process.
This change is