test(storage): add CRD tests #1952
Conversation
product-auto-label
bot
added
the
api: storage
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1952 +/- ##
=======================================
Coverage 95.67% 95.67%
=======================================
Files 57 57
Lines 2056 2056
=======================================
Hits 1967 1967
Misses 89 89 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| println!("\nTesting create_bucket()"); | ||
| let create = client | ||
| .create_bucket("projects/_", bucket_id) |
There was a problem hiding this comment.
optional: As you may recall, the rate limit for this call is 0.5Hz. We should to adjust the backoff to start a 2s, at least for this call.
There was a problem hiding this comment.
the rate limit for this call is 0.5Hz.
I did read that. I saw we ran CRUD tests for buckets in Cloud C++ so figured it couldn't be that bad to run them here.
adjust the backoff to start a 2s
Good idea. Done.
src/integration-tests/src/storage.rs
Outdated
| assert_eq!(get.name, bucket_name); | ||
|
|
||
| println!("\nTesting list_buckets()"); | ||
| let mut paginator = client |
There was a problem hiding this comment.
nit: I have taken to calling these "items"... they certainly are not a paginator after the items() call?
There was a problem hiding this comment.
Ack, I am going to call them bucket(s). Seems more specific.
src/integration-tests/src/storage.rs
Outdated
| "missing bucket name {} in {bucket_names:?}", | ||
| &bucket_name |
There was a problem hiding this comment.
nit:
| "missing bucket name {} in {bucket_names:?}", | |
| &bucket_name | |
| "missing bucket name {bucket_name} in {bucket_names:?}" |
|
|
||
| pub(crate) fn random_bucket_id() -> String { | ||
| const CHARSET: &[u8] = b"abcdefghijklmnopqrstuvwxyz0123456789"; | ||
| const PREFIX: &str = "rust-sdk-testing-"; |
There was a problem hiding this comment.
If we use the integration-tests label we do not need a long prefix like this.
There was a problem hiding this comment.
I like the prefix though. It makes it more obvious what these things are if they leak (especially in a testing project). I don't mind sacrificing some bits of randomness.
src/integration-tests/src/storage.rs
Outdated
| let mut bucket_id = String::new(); | ||
| for _ in 0..(BUCKET_ID_LENGTH - PREFIX.len()) { | ||
| let idx = rand::rng().random_range(0..CHARSET.len()); | ||
| bucket_id.push(CHARSET[idx] as char); | ||
| } |
There was a problem hiding this comment.
Consider:
| let mut bucket_id = String::new(); | |
| for _ in 0..(BUCKET_ID_LENGTH - PREFIX.len()) { | |
| let idx = rand::rng().random_range(0..CHARSET.len()); | |
| bucket_id.push(CHARSET[idx] as char); | |
| } | |
| let id = std::iter::from_fn(|| move { rand::rng().random_range(0..CHARSET.len()) }) | |
| .take(BUCKET_ID_LENGTH - PREFIX.len()) | |
| .map(|idx| char::from(CHARSET[idx]) | |
| .collect(); |
Or if you want to be nice, add an analog to Alphanumeric for lowercase characters to lib.rs.
There was a problem hiding this comment.
You are now the proud owner of RandomChars which impls rand::distr::Distribution<u8> for any set of characters we may care about.
Part of the work for #1813, will be useful for testing #1951
Add bucket CRUD tests minus the U.