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

fix: exclude backslashes from initially generated passwords #170

Merged
merged 2 commits into from
Feb 10, 2025

Conversation

nortaljevgenikr
Copy link
Contributor

Refs: OPMONDEV-192

@nortaljevgenikr nortaljevgenikr marked this pull request as ready for review February 7, 2025 13:47
@nortaljevgenikr nortaljevgenikr requested a review from raits February 7, 2025 13:47
Copy link
Contributor

@raits raits left a comment

Choose a reason for hiding this comment

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

In general it looks good to me, however I added a comment about testing this.

I think if we can't come up with a better alternative, I would just not test this case as it is too run dependent.

Comment on lines 139 to 141
for _ in range(1000):
password = create_users._generate_password()
assert backslash_char not in password
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand why this is tested like that, but perhaps there is a better way to test the behavior of the logic rather than the end result. To me this seems like it would make for a hard to understand sometimes failing / sometimes not test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point. What about now?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good.

Comment on lines 163 to 167
def test_generated_password_has_no_backslashes():
backslash_char = "\\"
for _ in range(1000):
password = create_users._generate_password()
assert backslash_char not in password
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as for the MongoDB one.

@nortaljevgenikr nortaljevgenikr requested a review from raits February 7, 2025 15:52
Copy link
Contributor

@raits raits left a comment

Choose a reason for hiding this comment

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

Looks good to me now.

@nortaljevgenikr nortaljevgenikr force-pushed the OPMONDEV-192_exclude_backslash branch from 6b842e9 to 1ab8c4b Compare February 10, 2025 07:08
@nordic-institute nordic-institute deleted a comment from sonarqubecloud bot Feb 10, 2025
@raits raits merged commit 8c100e6 into develop Feb 10, 2025
10 of 11 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