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.

Sign up for GitHub

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

Jump to bottom

add tests for password and fix an edge case #2741

Merged
merged 9 commits into from
Apr 24, 2023
Merged

add tests for password and fix an edge case #2741

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a3b150a
add test for password and fix an edge case
DeepakRajJaganathan Apr 5, 2023
3831c0a
fix invalid password case
DeepakRajJaganathan Apr 5, 2023
cb573da
fix invalid case
DeepakRajJaganathan Apr 5, 2023
3b428f0
update error assertion and change error message
DeepakRaj228 Apr 18, 2023
de3ca38
Merge branch 'main' into tests_for_password_generators
DeepakRaj228 Apr 18, 2023
353047d
fix min,max length error message
DeepakRaj228 Apr 19, 2023
2118003
Merge branch 'tests_for_password_generators' of github.51.al-deepak-per…
DeepakRaj228 Apr 19, 2023
dc7cc83
fix rubocop
DeepakRaj228 Apr 21, 2023
6a71ce5
fix rubocop
DeepakRaj228 Apr 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/faker/default/internet.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ def username(specifier: nil, separators: %w[. _])
# @faker.version 2.1.3
def password(min_length: 8, max_length: 16, mix_case: true, special_characters: false)
raise ArgumentError, 'max_length must be more than min_length' if max_length < min_length
raise ArgumentError, 'min_length and max_length must have at least one character' if(min_length == 0 && max_length == 0)
Copy link
Contributor

@thdaraujo thdaraujo Apr 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think the first assertion message is misleading as min and max could be equal.

If that is true, and we can allow min_length == max_length, then we could be a bit more descriptive, as the base case would be:
min_length == 1 && max_length == 1.

Otherwise, min_length <= max_length where min and max are greater than 1. What do you think?

Suggested change
raise ArgumentError, 'max_length must be more than min_length' if max_length < min_length
raise ArgumentError, 'min_length and max_length must have at least one character' if(min_length == 0 && max_length == 0)
raise ArgumentError, 'min_length and max_length must be greater than or equal to one' if min_length < 1 || max_length < 1
raise ArgumentError, 'min_length must be smaller than or equal to max_length' unless min_length <= max_length

Copy link
Contributor Author

@DeepakRaj228 DeepakRaj228 Apr 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@thdaraujo I agree with your point and the above suggestion is even clearer than the current one.

Copy link
Contributor

Choose a reason for hiding this comment

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

ohh, thank you @thdaraujo for catching that! 👀


character_types = []
required_min_length = 0
Expand All @@ -186,7 +187,7 @@ def password(min_length: 8, max_length: 16, mix_case: true, special_characters:
password << lower_chars[rand(lower_chars.count - 1)]
character_bag += lower_chars

digits = ('1'..'9').to_a
digits = ('0'..'9').to_a
DeepakRaj228 marked this conversation as resolved.
Show resolved Hide resolved
password << digits[rand(digits.count - 1)]
character_bag += digits

Expand Down
34 changes: 32 additions & 2 deletions test/faker/default/test_faker_internet.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,13 +170,16 @@ def test_username_with_range_and_separators
end
end

def test_password
def test_password_with_no_arguments
password = @tester.password
default_min_length = 8
default_max_length = 16

assert_match(/\w{3}/, password)
assert_match(/\d/, password)
assert_match(/[a-z]/, password)
assert_match(/[A-Z]/, password)
assert_match(/[0-9]/, password)
assert_includes((default_min_length..default_max_length), password.length, 'Generated password length is incorrect')
end

def test_password_with_integer_arg
Expand Down Expand Up @@ -233,6 +236,12 @@ def test_password_with_min_length_and_max_length
assert_includes (min_length..max_length), password.size, 'Password size is incorrect'
end

def test_password_with_same_min_max_length
password = @tester.password(min_length: 5, max_length: 5)
assert_match(/\w+/, password)
assert_equal(5, password.length)
end

def test_password_with_max_length_less_than_min_length
assert_raise 'max_length must be more than min_length' do
@tester.password(min_length: 8, max_length: 4)
Expand Down Expand Up @@ -282,6 +291,27 @@ def test_password_with_invalid_min_length_for_mix_case_and_special_characters
end
end

def test_password_with_invalid_min_max_length
error = assert_raises(ArgumentError) do
@tester.password(min_length: 0, max_length: 0, mix_case: false, special_characters: false)
end
assert_equal 'min_length and max_length must have at least one character', error.message
end

def test_password_with_invalid_min_length_for_special_characters_only
error = assert_raises(ArgumentError) do
@tester.password(min_length: 0, mix_case: false, special_characters: true)
end
assert_equal 'min_length should be at least 1 to enable special_characters configuration', error.message
end

def test_password_with_invalid_min_length_for_mix_case_only
DeepakRaj228 marked this conversation as resolved.
Show resolved Hide resolved
error = assert_raises(ArgumentError) do
@tester.password(min_length: 1, mix_case: true)
end
assert_equal 'min_length should be at least 2 to enable mix_case configuration', error.message
end

def test_password_with_compatible_min_length_and_requirements
assert_nothing_raised do
[false, true].each do |value|
Expand Down