-
Notifications
You must be signed in to change notification settings - Fork 1.3k
CSHARP-4386: Test mongocryptd is not spawned when shared library is loaded. #965
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
Conversation
var clientEncryptedSchema = new BsonDocument("db.coll", JsonFileReader.Instance.Documents["external.external-schema.json"]); | ||
var cryptSharedPath = CoreTestConfiguration.GetCryptSharedLibPath(); | ||
Ensure.That(File.Exists(cryptSharedPath), $"Shared library path {cryptSharedPath} is not valid."); | ||
var effectiveExtraOptions = new Dictionary<string, object>(extraOptions) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where effectiveExtraOptions
is used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point, fixed. The test actually behaves almost the same even without this configuration because a shared library is configured by default, but it's definitely better to have it configured explicitly.
}; | ||
return ConfigureClientEncrypted(kmsProviderFilter: kmsProvider, schemaMap: clientEncryptedSchema, extraOptions: extraOptions); | ||
} | ||
default: throw new Exception($"Invalid bypass mongocryptd {bypassSpawning} option."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: ArgumentOutOfRangeException
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this file, we use Exception
in other places. I don't have strong preference in general what exception better to use, but mostly in tests we use just Exception
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.