-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Let SecretStrs be value equals #1079
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1079 +/- ##
======================================
Coverage 100% 100%
======================================
Files 20 20
Lines 3365 3369 +4
Branches 663 663
======================================
+ Hits 3365 3369 +4
Continue to review full report at Codecov.
|
I couldnt find a place that needed documentation updating but correct me if I'm wrong |
@@ -1699,6 +1699,10 @@ class Foobar(BaseModel): | |||
with pytest.warns(DeprecationWarning, match=r'`secret_str.display\(\)` is deprecated'): | |||
assert f.empty_password.display() == '' | |||
|
|||
# Assert that SecretStr is equal to SecretStr if the secret is the same. | |||
assert f == f.copy() | |||
assert f != f.copy(update=dict(password='4321')) |
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.
this is fine, but please add an explicit test, e.g.:
def test_secret_str():
assert SecretStr('123') == SecretStr('123')
assert SecretStr('123') != SecretStr('321')
assert SecretStr('123') != '123'
assert SecretStr('123') is not SecretStr('123')
And the same for SecretBytes
.
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.
I kept the copy version and added these below. Ok?
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.
just a tweak to separate tests.
thanks a lot. |
Change Summary
As discussed in the issue, this PR adds a
__eq__
method for SecretStr that compares the secret value such that SecretStr('a') == SecretStr('a') and SecretStr('a') != SecretStr('b')Related issue number
#1078
Checklist
changes/<pull request or issue id>-<github username>.md
file added describing change(see changes/README.md for details)