Skip to content

RUST-2080 spec tests (and fix) for gridfs rename #1366

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 1, 2025

Conversation

abr-egn
Copy link
Contributor

@abr-egn abr-egn commented Apr 29, 2025

RUST-2080

This syncs the tests and adds the missing file-not-found logic that the tests caught.

This did require making the rename test operation a little odd because it can either target a collection, in which case it accepts arguments of the form { to: String }, or it can target a gridfs bucket, where the arguments are { id: Bson, newFilename: String }. We parse the test file json into TestOperation values at file load time, where entity type isn't known, so we can't have distinct operation types for "rename collection" and "rename bucket", so instead there's now a wrapper layer that defers parsing the args document until the test is actually executed at which point it checks entity type and parses appropriately.

@abr-egn abr-egn marked this pull request as ready for review April 30, 2025 18:46
@abr-egn abr-egn requested a review from a team as a code owner April 30, 2025 18:46
@abr-egn abr-egn requested review from isabelatkinson and removed request for a team April 30, 2025 18:46
@abr-egn abr-egn force-pushed the RUST-2080/gridfs-rename-tests branch from 0a99647 to 8c0dc9f Compare April 30, 2025 20:25
Comment on lines +57 to +62
let cmd = doc! {
"renameCollection": crate::bson::to_bson(&ns)?,
"to": crate::bson::to_bson(&to_ns)?,
};
let admin = test_runner.internal_client.database("admin");
admin.run_command(cmd).await?;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could technically drop support for this operation and skip tests that use it since it's not part of our API. That said, I can't think of a good reason to reduce test coverage here so probably fine to leave as-is

@abr-egn abr-egn merged commit 7208462 into mongodb:main May 1, 2025
16 of 18 checks passed
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