Skip to content
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

GODRIVER-3140 Update client-side-encryption spec tests. #1651

Draft
wants to merge 10 commits into
base: v1
Choose a base branch
from

Conversation

qingyang-hu
Copy link
Collaborator

@qingyang-hu qingyang-hu commented May 31, 2024

GODRIVER-3140

Summary

Update client-side-encryption spec tests, reflecting spec PR #1564.

@mongodb-drivers-pr-bot mongodb-drivers-pr-bot bot added the priority-3-low Low Priority PR for Review label May 31, 2024
Copy link
Contributor

mongodb-drivers-pr-bot bot commented May 31, 2024

API Change Report

No changes found!

@qingyang-hu qingyang-hu force-pushed the godriver3140 branch 3 times, most recently from 475502f to 96298c3 Compare June 1, 2024 04:09
return nil
}
}
return fmt.Errorf("BSON type mismatch for key %s; expected %s, got %s", key, array.String(), actual)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
return fmt.Errorf("BSON type mismatch for key %s; expected %s, got %s", key, array.String(), actual)
return fmt.Errorf("BSON type mismatch for key %q; expected %s, got %q", key, array, actual.Type)

%s will use the bson.RawValue.String() method for array. Additionally, suggest using the actual type rather than the value of actual in the error message.

}
return fmt.Errorf("BSON type mismatch for key %s; expected %s, got %s", key, array.String(), actual)
default:
return fmt.Errorf("unsupported $$type: %s", t.String())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
return fmt.Errorf("unsupported $$type: %s", t.String())
return fmt.Errorf("unsupported $$type: %q", t)

q will use the bsontype String() method.

@@ -82,7 +82,24 @@ func compareValues(mt *mtest.T, key string, expected, actual bson.RawValue) erro
if typeVal, err := e.LookupErr("$$type"); err == nil {
// $$type represents a type assertion
// for example {field: {$$type: "binData"}} should assert that "field" is an element with a binary value
return checkValueType(mt, key, actual.Type, typeVal.StringValue())
switch t := typeVal.Type; t {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Optional. Use a different name for this variable, such as typ.

prestonvasquez
prestonvasquez previously approved these changes Jun 11, 2024
Comment on lines 589 to 591
if t.clientOpts != nil && t.clientOpts.AutoEncryptionOptions != nil && len(t.failPointNames) > 0 {
t.Logf("configureFailPoint is not supported for auto encryption, skipping ClearFailPoints()")
t.failPointNames = t.failPointNames[:0]
Copy link
Collaborator

Choose a reason for hiding this comment

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

The added spec tests include both AutoEncryptionOptions and "configureFailPoint", which seems to suggest that "configureFailPoint" is being run successfully during the test (to enable the failpoint). Does disabling failpoints behave differently than enabling failpoints when AutoEncryptionOptions are set?

Copy link
Collaborator Author

@qingyang-hu qingyang-hu Jun 18, 2024

Choose a reason for hiding this comment

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

Yes, clearing fail point fails for "mongocrypt error 1: command not supported for auto encryption: configureFailPoint".
^ The issue was caused because of GODRIVER-2123.

@qingyang-hu qingyang-hu force-pushed the godriver3140 branch 2 times, most recently from a859b1e to 80e941f Compare August 8, 2024 19:04
@qingyang-hu qingyang-hu marked this pull request as ready for review August 8, 2024 22:21
@qingyang-hu qingyang-hu marked this pull request as draft August 8, 2024 23:37
@qingyang-hu qingyang-hu force-pushed the godriver3140 branch 2 times, most recently from 17c4846 to 12fb07c Compare August 15, 2024 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority-3-low Low Priority PR for Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants