Magic number in tests

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

Magic number in tests

Artur Beljajev
Magic number 72 is used many times throughout the tests, which is actually present under ActiveModel::SecurePassword::MAX_PASSWORD_LENGTH_ALLOWED constant. I propose to replace that magic number either by the constant or method call.

I have never contributed to such large open source project as Rails. Please let me know your thoughts.
 


https://github.com/rails/rails/blob/master/activemodel/test/cases/secure_password_test.rb
https://github.com/rails/rails/blob/ff657e73f0b61a7e224a9f158467ed2ec915bd9b/activemodel/lib/active_model/secure_password.rb

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Magic number in tests

Kasper Timm Hansen
I’m -1 on changing that. I think the constant 72 is used directly such that tests will fail if anybody changes MAX_PASSWORD_LENGTH_ALLOWED.

With your change in, a later change to MAX_PASSWORD_LENGTH_ALLOWED by some other contributor wouldn’t make any tests fail.

Since that constant isn’t meant to change without us thinking more about it, I’d say we keep the code as is.

Thanks for starting to contribute!

Den 9. jan. 2018 kl. 09.56 skrev [hidden email]:

Magic number 72 is used many times throughout the tests, which is actually present under ActiveModel::SecurePassword::MAX_PASSWORD_LENGTH_ALLOWED constant. I propose to replace that magic number either by the constant or method call.


--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

--
Kasper

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Magic number in tests

Kerri Miller
I could see an argument made for making it a constant in the _test_ (EXPECTED_MAX_PASSWORD_LENGTH), which at least would document /why/ the number is being used.

On Tue, Jan 9, 2018 at 11:37 AM, Kasper Timm Hansen <[hidden email]> wrote:
I’m -1 on changing that. I think the constant 72 is used directly such that tests will fail if anybody changes MAX_PASSWORD_LENGTH_ALLOWED.

With your change in, a later change to MAX_PASSWORD_LENGTH_ALLOWED by some other contributor wouldn’t make any tests fail.

Since that constant isn’t meant to change without us thinking more about it, I’d say we keep the code as is.

Thanks for starting to contribute!

Den 9. jan. 2018 kl. 09.56 skrev [hidden email]:

Magic number 72 is used many times throughout the tests, which is actually present under ActiveModel::SecurePassword::MAX_PASSWORD_LENGTH_ALLOWED constant. I propose to replace that magic number either by the constant or method call.


--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

--
Kasper

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Magic number in tests

Kasper Timm Hansen
Yep, we could do that 👍

Den 9. jan. 2018 kl. 12.39 skrev Kerri Miller <[hidden email]>:

I could see an argument made for making it a constant in the _test_ (EXPECTED_MAX_PASSWORD_LENGTH), which at least would document /why/ the number is being used.

On Tue, Jan 9, 2018 at 11:37 AM, Kasper Timm Hansen <[hidden email]> wrote:
I’m -1 on changing that. I think the constant 72 is used directly such that tests will fail if anybody changes MAX_PASSWORD_LENGTH_ALLOWED.

With your change in, a later change to MAX_PASSWORD_LENGTH_ALLOWED by some other contributor wouldn’t make any tests fail.

Since that constant isn’t meant to change without us thinking more about it, I’d say we keep the code as is.

Thanks for starting to contribute!

Den 9. jan. 2018 kl. 09.56 skrev [hidden email]:

Magic number 72 is used many times throughout the tests, which is actually present under ActiveModel::SecurePassword::MAX_PASSWORD_LENGTH_ALLOWED constant. I propose to replace that magic number either by the constant or method call.


--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

--
Kasper


--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.


--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

--
Kasper

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Magic number in tests

Artur Beljajev
I would be happy to do this!


On Tuesday, January 9, 2018 at 10:30:41 PM UTC+2, Kasper Timm Hansen wrote:
Yep, we could do that 👍

Den 9. jan. 2018 kl. 12.39 skrev Kerri Miller <<a href="javascript:" target="_blank" gdf-obfuscated-mailto="bFcG683bDgAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">kerr...@...>:

I could see an argument made for making it a constant in the _test_ (EXPECTED_MAX_PASSWORD_LENGTH), which at least would document /why/ the number is being used.

On Tue, Jan 9, 2018 at 11:37 AM, Kasper Timm Hansen <<a href="javascript:" target="_blank" gdf-obfuscated-mailto="bFcG683bDgAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">kas...@...> wrote:
I’m -1 on changing that. I think the constant 72 is used directly such that tests will fail if anybody changes MAX_PASSWORD_LENGTH_ALLOWED.

With your change in, a later change to MAX_PASSWORD_LENGTH_ALLOWED by some other contributor wouldn’t make any tests fail.

Since that constant isn’t meant to change without us thinking more about it, I’d say we keep the code as is.

Thanks for starting to contribute!

Den 9. jan. 2018 kl. 09.56 skrev <a href="javascript:" target="_blank" gdf-obfuscated-mailto="bFcG683bDgAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">artur.b...@...:

Magic number 72 is used many times throughout the tests, which is actually present under ActiveModel::SecurePassword::MAX_PASSWORD_LENGTH_ALLOWED constant. I propose to replace that magic number either by the constant or method call.

I have never contributed to such large open source project as Rails. Please let me know your thoughts.
 


<a href="https://github.com/rails/rails/blob/master/activemodel/test/cases/secure_password_test.rb" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fblob%2Fmaster%2Factivemodel%2Ftest%2Fcases%2Fsecure_password_test.rb\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNGaLKGr9A14qSiG3WQraGuHgwAvMw&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fblob%2Fmaster%2Factivemodel%2Ftest%2Fcases%2Fsecure_password_test.rb\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNGaLKGr9A14qSiG3WQraGuHgwAvMw&#39;;return true;">https://github.com/rails/rails/blob/master/activemodel/test/cases/secure_password_test.rb
<a href="https://github.com/rails/rails/blob/ff657e73f0b61a7e224a9f158467ed2ec915bd9b/activemodel/lib/active_model/secure_password.rb" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fblob%2Fff657e73f0b61a7e224a9f158467ed2ec915bd9b%2Factivemodel%2Flib%2Factive_model%2Fsecure_password.rb\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEWQruGyMZXbRw6FLmz6Fk9ddtpyA&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fblob%2Fff657e73f0b61a7e224a9f158467ed2ec915bd9b%2Factivemodel%2Flib%2Factive_model%2Fsecure_password.rb\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEWQruGyMZXbRw6FLmz6Fk9ddtpyA&#39;;return true;">https://github.com/rails/rails/blob/ff657e73f0b61a7e224a9f158467ed2ec915bd9b/activemodel/lib/active_model/secure_password.rb

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to <a href="javascript:" target="_blank" gdf-obfuscated-mailto="bFcG683bDgAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">rubyonrails-co...@googlegroups.com.
To post to this group, send email to <a href="javascript:" target="_blank" gdf-obfuscated-mailto="bFcG683bDgAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">rubyonra...@googlegroups.com.
Visit this group at <a href="https://groups.google.com/group/rubyonrails-core" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;">https://groups.google.com/group/rubyonrails-core.
For more options, visit <a href="https://groups.google.com/d/optout" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;">https://groups.google.com/d/optout.

--
Kasper


--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to <a href="javascript:" target="_blank" gdf-obfuscated-mailto="bFcG683bDgAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">rubyonrails-co...@googlegroups.com.
To post to this group, send email to <a href="javascript:" target="_blank" gdf-obfuscated-mailto="bFcG683bDgAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">rubyonra...@googlegroups.com.
Visit this group at <a href="https://groups.google.com/group/rubyonrails-core" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;">https://groups.google.com/group/rubyonrails-core.
For more options, visit <a href="https://groups.google.com/d/optout" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;">https://groups.google.com/d/optout.


--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to <a href="javascript:" target="_blank" gdf-obfuscated-mailto="bFcG683bDgAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">rubyonrails-co...@googlegroups.com.
To post to this group, send email to <a href="javascript:" target="_blank" gdf-obfuscated-mailto="bFcG683bDgAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">rubyonra...@googlegroups.com.
Visit this group at <a href="https://groups.google.com/group/rubyonrails-core" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;">https://groups.google.com/group/rubyonrails-core.
For more options, visit <a href="https://groups.google.com/d/optout" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;">https://groups.google.com/d/optout.

--
Kasper

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Magic number in tests

Artur Beljajev
Thank you for feedback!

On Wednesday, January 10, 2018 at 4:19:44 PM UTC+2, [hidden email] wrote:
I would be happy to do this!


On Tuesday, January 9, 2018 at 10:30:41 PM UTC+2, Kasper Timm Hansen wrote:
Yep, we could do that 👍

Den 9. jan. 2018 kl. 12.39 skrev Kerri Miller <[hidden email]>:

I could see an argument made for making it a constant in the _test_ (EXPECTED_MAX_PASSWORD_LENGTH), which at least would document /why/ the number is being used.

On Tue, Jan 9, 2018 at 11:37 AM, Kasper Timm Hansen <[hidden email]> wrote:
I’m -1 on changing that. I think the constant 72 is used directly such that tests will fail if anybody changes MAX_PASSWORD_LENGTH_ALLOWED.

With your change in, a later change to MAX_PASSWORD_LENGTH_ALLOWED by some other contributor wouldn’t make any tests fail.

Since that constant isn’t meant to change without us thinking more about it, I’d say we keep the code as is.

Thanks for starting to contribute!

Den 9. jan. 2018 kl. 09.56 skrev [hidden email]:

Magic number 72 is used many times throughout the tests, which is actually present under ActiveModel::SecurePassword::MAX_PASSWORD_LENGTH_ALLOWED constant. I propose to replace that magic number either by the constant or method call.

I have never contributed to such large open source project as Rails. Please let me know your thoughts.
 


<a href="https://github.com/rails/rails/blob/master/activemodel/test/cases/secure_password_test.rb" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fblob%2Fmaster%2Factivemodel%2Ftest%2Fcases%2Fsecure_password_test.rb\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNGaLKGr9A14qSiG3WQraGuHgwAvMw&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fblob%2Fmaster%2Factivemodel%2Ftest%2Fcases%2Fsecure_password_test.rb\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNGaLKGr9A14qSiG3WQraGuHgwAvMw&#39;;return true;">https://github.com/rails/rails/blob/master/activemodel/test/cases/secure_password_test.rb
<a href="https://github.com/rails/rails/blob/ff657e73f0b61a7e224a9f158467ed2ec915bd9b/activemodel/lib/active_model/secure_password.rb" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fblob%2Fff657e73f0b61a7e224a9f158467ed2ec915bd9b%2Factivemodel%2Flib%2Factive_model%2Fsecure_password.rb\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEWQruGyMZXbRw6FLmz6Fk9ddtpyA&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fblob%2Fff657e73f0b61a7e224a9f158467ed2ec915bd9b%2Factivemodel%2Flib%2Factive_model%2Fsecure_password.rb\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEWQruGyMZXbRw6FLmz6Fk9ddtpyA&#39;;return true;">https://github.com/rails/rails/blob/ff657e73f0b61a7e224a9f158467ed2ec915bd9b/activemodel/lib/active_model/secure_password.rb

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to [hidden email].
Visit this group at <a href="https://groups.google.com/group/rubyonrails-core" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;">https://groups.google.com/group/rubyonrails-core.
For more options, visit <a href="https://groups.google.com/d/optout" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;">https://groups.google.com/d/optout.

--
Kasper


--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to [hidden email].
Visit this group at <a href="https://groups.google.com/group/rubyonrails-core" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;">https://groups.google.com/group/rubyonrails-core.
For more options, visit <a href="https://groups.google.com/d/optout" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;">https://groups.google.com/d/optout.


--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to [hidden email].
Visit this group at <a href="https://groups.google.com/group/rubyonrails-core" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;">https://groups.google.com/group/rubyonrails-core.
For more options, visit <a href="https://groups.google.com/d/optout" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;">https://groups.google.com/d/optout.

--
Kasper

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Magic number in tests

Kerri Miller
In reply to this post by Artur Beljajev
Not to "well actually" myself but another option might be to use MAX_PASSWORD_LENGTH_ALLOWED as the OP suggested, but have an explicit test for MAX_PASSWORD_LENGTH_ALLOWED=72.. which might actually be a better approach, since it'll isolate the exact defect case (MAX_PASSWORD_LENGTH_ALLOWED != 72) rather than have it be implied by a well-named-but-oddly-disconnected constant.

// Kerri

On Wed, Jan 10, 2018 at 3:19 PM, <[hidden email]> wrote:
I would be happy to do this!


On Tuesday, January 9, 2018 at 10:30:41 PM UTC+2, Kasper Timm Hansen wrote:
Yep, we could do that 👍

Den 9. jan. 2018 kl. 12.39 skrev Kerri Miller <[hidden email]>:

I could see an argument made for making it a constant in the _test_ (EXPECTED_MAX_PASSWORD_LENGTH), which at least would document /why/ the number is being used.

On Tue, Jan 9, 2018 at 11:37 AM, Kasper Timm Hansen <[hidden email]> wrote:
I’m -1 on changing that. I think the constant 72 is used directly such that tests will fail if anybody changes MAX_PASSWORD_LENGTH_ALLOWED.

With your change in, a later change to MAX_PASSWORD_LENGTH_ALLOWED by some other contributor wouldn’t make any tests fail.

Since that constant isn’t meant to change without us thinking more about it, I’d say we keep the code as is.

Thanks for starting to contribute!

Den 9. jan. 2018 kl. 09.56 skrev [hidden email]:

Magic number 72 is used many times throughout the tests, which is actually present under ActiveModel::SecurePassword::MAX_PASSWORD_LENGTH_ALLOWED constant. I propose to replace that magic number either by the constant or method call.


--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

--
Kasper


--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.


--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

--
Kasper

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Magic number in tests

Kasper Timm Hansen
Yep, that sounds good too, Kerri.

Artur, go right ahead!

--
Kasper

On 10 Jan 2018, 15.37 +0100, Kerri Miller <[hidden email]>, wrote:
Not to "well actually" myself but another option might be to use MAX_PASSWORD_LENGTH_ALLOWED as the OP suggested, but have an explicit test for MAX_PASSWORD_LENGTH_ALLOWED=72.. which might actually be a better approach, since it'll isolate the exact defect case (MAX_PASSWORD_LENGTH_ALLOWED != 72) rather than have it be implied by a well-named-but-oddly-disconnected constant.

// Kerri

On Wed, Jan 10, 2018 at 3:19 PM, <[hidden email]> wrote:
I would be happy to do this!


On Tuesday, January 9, 2018 at 10:30:41 PM UTC+2, Kasper Timm Hansen wrote:
Yep, we could do that 👍

Den 9. jan. 2018 kl. 12.39 skrev Kerri Miller <[hidden email]>:

I could see an argument made for making it a constant in the _test_ (EXPECTED_MAX_PASSWORD_LENGTH), which at least would document /why/ the number is being used.

On Tue, Jan 9, 2018 at 11:37 AM, Kasper Timm Hansen <[hidden email]> wrote:
I’m -1 on changing that. I think the constant 72 is used directly such that tests will fail if anybody changes MAX_PASSWORD_LENGTH_ALLOWED.

With your change in, a later change to MAX_PASSWORD_LENGTH_ALLOWED by some other contributor wouldn’t make any tests fail.

Since that constant isn’t meant to change without us thinking more about it, I’d say we keep the code as is.

Thanks for starting to contribute!

Den 9. jan. 2018 kl. 09.56 skrev [hidden email]:

Magic number 72 is used many times throughout the tests, which is actually present under ActiveModel::SecurePassword::MAX_PASSWORD_LENGTH_ALLOWED constant. I propose to replace that magic number either by the constant or method call.


--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

--
Kasper


--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.


--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

--
Kasper

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Magic number in tests

Artur Beljajev
Should I create a ticket on Github first?

On Saturday, January 20, 2018 at 3:28:49 PM UTC+2, Kasper Timm Hansen wrote:
Yep, that sounds good too, Kerri.

Artur, go right ahead!

--
Kasper

On 10 Jan 2018, 15.37 +0100, Kerri Miller <<a href="javascript:" target="_blank" gdf-obfuscated-mailto="PGJjJIF-AgAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">kerr...@...>, wrote:
Not to "well actually" myself but another option might be to use MAX_PASSWORD_LENGTH_ALLOWED as the OP suggested, but have an explicit test for MAX_PASSWORD_LENGTH_ALLOWED=72.. which might actually be a better approach, since it'll isolate the exact defect case (MAX_PASSWORD_LENGTH_ALLOWED != 72) rather than have it be implied by a well-named-but-oddly-disconnected constant.

// Kerri

On Wed, Jan 10, 2018 at 3:19 PM, <<a href="javascript:" target="_blank" gdf-obfuscated-mailto="PGJjJIF-AgAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">artur.b...@...> wrote:
I would be happy to do this!


On Tuesday, January 9, 2018 at 10:30:41 PM UTC+2, Kasper Timm Hansen wrote:
Yep, we could do that 👍

Den 9. jan. 2018 kl. 12.39 skrev Kerri Miller <[hidden email]>:

I could see an argument made for making it a constant in the _test_ (EXPECTED_MAX_PASSWORD_LENGTH), which at least would document /why/ the number is being used.

On Tue, Jan 9, 2018 at 11:37 AM, Kasper Timm Hansen <[hidden email]> wrote:
I’m -1 on changing that. I think the constant 72 is used directly such that tests will fail if anybody changes MAX_PASSWORD_LENGTH_ALLOWED.

With your change in, a later change to MAX_PASSWORD_LENGTH_ALLOWED by some other contributor wouldn’t make any tests fail.

Since that constant isn’t meant to change without us thinking more about it, I’d say we keep the code as is.

Thanks for starting to contribute!

Den 9. jan. 2018 kl. 09.56 skrev [hidden email]:

Magic number 72 is used many times throughout the tests, which is actually present under ActiveModel::SecurePassword::MAX_PASSWORD_LENGTH_ALLOWED constant. I propose to replace that magic number either by the constant or method call.

I have never contributed to such large open source project as Rails. Please let me know your thoughts.
 


<a href="https://github.com/rails/rails/blob/master/activemodel/test/cases/secure_password_test.rb" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fblob%2Fmaster%2Factivemodel%2Ftest%2Fcases%2Fsecure_password_test.rb\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNGaLKGr9A14qSiG3WQraGuHgwAvMw&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fblob%2Fmaster%2Factivemodel%2Ftest%2Fcases%2Fsecure_password_test.rb\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNGaLKGr9A14qSiG3WQraGuHgwAvMw&#39;;return true;">https://github.com/rails/rails/blob/master/activemodel/test/cases/secure_password_test.rb
<a href="https://github.com/rails/rails/blob/ff657e73f0b61a7e224a9f158467ed2ec915bd9b/activemodel/lib/active_model/secure_password.rb" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fblob%2Fff657e73f0b61a7e224a9f158467ed2ec915bd9b%2Factivemodel%2Flib%2Factive_model%2Fsecure_password.rb\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEWQruGyMZXbRw6FLmz6Fk9ddtpyA&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fblob%2Fff657e73f0b61a7e224a9f158467ed2ec915bd9b%2Factivemodel%2Flib%2Factive_model%2Fsecure_password.rb\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEWQruGyMZXbRw6FLmz6Fk9ddtpyA&#39;;return true;">https://github.com/rails/rails/blob/ff657e73f0b61a7e224a9f158467ed2ec915bd9b/activemodel/lib/active_model/secure_password.rb

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to [hidden email].
Visit this group at <a href="https://groups.google.com/group/rubyonrails-core" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;">https://groups.google.com/group/rubyonrails-core.
For more options, visit <a href="https://groups.google.com/d/optout" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;">https://groups.google.com/d/optout.

--
Kasper


--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to [hidden email].
Visit this group at <a href="https://groups.google.com/group/rubyonrails-core" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;">https://groups.google.com/group/rubyonrails-core.
For more options, visit <a href="https://groups.google.com/d/optout" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;">https://groups.google.com/d/optout.


--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to [hidden email].
Visit this group at <a href="https://groups.google.com/group/rubyonrails-core" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;">https://groups.google.com/group/rubyonrails-core.
For more options, visit <a href="https://groups.google.com/d/optout" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;">https://groups.google.com/d/optout.

--
Kasper

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to <a href="javascript:" target="_blank" gdf-obfuscated-mailto="PGJjJIF-AgAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">rubyonrails-co...@googlegroups.com.
To post to this group, send email to <a href="javascript:" target="_blank" gdf-obfuscated-mailto="PGJjJIF-AgAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">rubyonra...@googlegroups.com.
Visit this group at <a href="https://groups.google.com/group/rubyonrails-core" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;">https://groups.google.com/group/rubyonrails-core.
For more options, visit <a href="https://groups.google.com/d/optout" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;">https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to <a href="javascript:" target="_blank" gdf-obfuscated-mailto="PGJjJIF-AgAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">rubyonrails-co...@googlegroups.com.
To post to this group, send email to <a href="javascript:" target="_blank" gdf-obfuscated-mailto="PGJjJIF-AgAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">rubyonra...@googlegroups.com.
Visit this group at <a href="https://groups.google.com/group/rubyonrails-core" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;">https://groups.google.com/group/rubyonrails-core.
For more options, visit <a href="https://groups.google.com/d/optout" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;">https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Magic number in tests

Kasper Timm Hansen
Artur, send a pull request :)

--
Kasper

On 21 Feb 2018, 01.14 +0100, Artur Beljajev <[hidden email]>, wrote:
Should I create a ticket on Github first?

On Saturday, January 20, 2018 at 3:28:49 PM UTC+2, Kasper Timm Hansen wrote:
Yep, that sounds good too, Kerri.

Artur, go right ahead!

--
Kasper

On 10 Jan 2018, 15.37 +0100, Kerri Miller <<a href="javascript:" target="_blank" gdf-obfuscated-mailto="PGJjJIF-AgAJ" rel="nofollow" onmousedown="this.href='javascript:';return true;" onclick="this.href='javascript:';return true;">kerr...@...>, wrote:
Not to "well actually" myself but another option might be to use MAX_PASSWORD_LENGTH_ALLOWED as the OP suggested, but have an explicit test for MAX_PASSWORD_LENGTH_ALLOWED=72.. which might actually be a better approach, since it'll isolate the exact defect case (MAX_PASSWORD_LENGTH_ALLOWED != 72) rather than have it be implied by a well-named-but-oddly-disconnected constant.

// Kerri

On Wed, Jan 10, 2018 at 3:19 PM, <<a href="javascript:" target="_blank" gdf-obfuscated-mailto="PGJjJIF-AgAJ" rel="nofollow" onmousedown="this.href='javascript:';return true;" onclick="this.href='javascript:';return true;">artur.b...@...> wrote:
I would be happy to do this!


On Tuesday, January 9, 2018 at 10:30:41 PM UTC+2, Kasper Timm Hansen wrote:
Yep, we could do that 👍

Den 9. jan. 2018 kl. 12.39 skrev Kerri Miller <[hidden email]>:

I could see an argument made for making it a constant in the _test_ (EXPECTED_MAX_PASSWORD_LENGTH), which at least would document /why/ the number is being used.

On Tue, Jan 9, 2018 at 11:37 AM, Kasper Timm Hansen <[hidden email]> wrote:
I’m -1 on changing that. I think the constant 72 is used directly such that tests will fail if anybody changes MAX_PASSWORD_LENGTH_ALLOWED.

With your change in, a later change to MAX_PASSWORD_LENGTH_ALLOWED by some other contributor wouldn’t make any tests fail.

Since that constant isn’t meant to change without us thinking more about it, I’d say we keep the code as is.

Thanks for starting to contribute!

Den 9. jan. 2018 kl. 09.56 skrev [hidden email]:

Magic number 72 is used many times throughout the tests, which is actually present under ActiveModel::SecurePassword::MAX_PASSWORD_LENGTH_ALLOWED constant. I propose to replace that magic number either by the constant or method call.

I have never contributed to such large open source project as Rails. Please let me know your thoughts.
 


<a href="https://github.com/rails/rails/blob/master/activemodel/test/cases/secure_password_test.rb" rel="nofollow" target="_blank" onmousedown="this.href='https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fblob%2Fmaster%2Factivemodel%2Ftest%2Fcases%2Fsecure_password_test.rb\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNGaLKGr9A14qSiG3WQraGuHgwAvMw';return true;" onclick="this.href='https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fblob%2Fmaster%2Factivemodel%2Ftest%2Fcases%2Fsecure_password_test.rb\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNGaLKGr9A14qSiG3WQraGuHgwAvMw';return true;">https://github.com/rails/rails/blob/master/activemodel/test/cases/secure_password_test.rb
<a href="https://github.com/rails/rails/blob/ff657e73f0b61a7e224a9f158467ed2ec915bd9b/activemodel/lib/active_model/secure_password.rb" rel="nofollow" target="_blank" onmousedown="this.href='https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fblob%2Fff657e73f0b61a7e224a9f158467ed2ec915bd9b%2Factivemodel%2Flib%2Factive_model%2Fsecure_password.rb\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEWQruGyMZXbRw6FLmz6Fk9ddtpyA';return true;" onclick="this.href='https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fblob%2Fff657e73f0b61a7e224a9f158467ed2ec915bd9b%2Factivemodel%2Flib%2Factive_model%2Fsecure_password.rb\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEWQruGyMZXbRw6FLmz6Fk9ddtpyA';return true;">https://github.com/rails/rails/blob/ff657e73f0b61a7e224a9f158467ed2ec915bd9b/activemodel/lib/active_model/secure_password.rb

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to [hidden email].
Visit this group at <a href="https://groups.google.com/group/rubyonrails-core" rel="nofollow" target="_blank" onmousedown="this.href='https://groups.google.com/group/rubyonrails-core';return true;" onclick="this.href='https://groups.google.com/group/rubyonrails-core';return true;">https://groups.google.com/group/rubyonrails-core.
For more options, visit <a href="https://groups.google.com/d/optout" rel="nofollow" target="_blank" onmousedown="this.href='https://groups.google.com/d/optout';return true;" onclick="this.href='https://groups.google.com/d/optout';return true;">https://groups.google.com/d/optout.

--
Kasper


--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to [hidden email].
Visit this group at <a href="https://groups.google.com/group/rubyonrails-core" rel="nofollow" target="_blank" onmousedown="this.href='https://groups.google.com/group/rubyonrails-core';return true;" onclick="this.href='https://groups.google.com/group/rubyonrails-core';return true;">https://groups.google.com/group/rubyonrails-core.
For more options, visit <a href="https://groups.google.com/d/optout" rel="nofollow" target="_blank" onmousedown="this.href='https://groups.google.com/d/optout';return true;" onclick="this.href='https://groups.google.com/d/optout';return true;">https://groups.google.com/d/optout.


--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to [hidden email].
Visit this group at <a href="https://groups.google.com/group/rubyonrails-core" rel="nofollow" target="_blank" onmousedown="this.href='https://groups.google.com/group/rubyonrails-core';return true;" onclick="this.href='https://groups.google.com/group/rubyonrails-core';return true;">https://groups.google.com/group/rubyonrails-core.
For more options, visit <a href="https://groups.google.com/d/optout" rel="nofollow" target="_blank" onmousedown="this.href='https://groups.google.com/d/optout';return true;" onclick="this.href='https://groups.google.com/d/optout';return true;">https://groups.google.com/d/optout.

--
Kasper

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to <a href="javascript:" target="_blank" gdf-obfuscated-mailto="PGJjJIF-AgAJ" rel="nofollow" onmousedown="this.href='javascript:';return true;" onclick="this.href='javascript:';return true;">rubyonrails-co...@googlegroups.com.
To post to this group, send email to <a href="javascript:" target="_blank" gdf-obfuscated-mailto="PGJjJIF-AgAJ" rel="nofollow" onmousedown="this.href='javascript:';return true;" onclick="this.href='javascript:';return true;">rubyonra...@googlegroups.com.
Visit this group at <a href="https://groups.google.com/group/rubyonrails-core" target="_blank" rel="nofollow" onmousedown="this.href='https://groups.google.com/group/rubyonrails-core';return true;" onclick="this.href='https://groups.google.com/group/rubyonrails-core';return true;">https://groups.google.com/group/rubyonrails-core.
For more options, visit <a href="https://groups.google.com/d/optout" target="_blank" rel="nofollow" onmousedown="this.href='https://groups.google.com/d/optout';return true;" onclick="this.href='https://groups.google.com/d/optout';return true;">https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to <a href="javascript:" target="_blank" gdf-obfuscated-mailto="PGJjJIF-AgAJ" rel="nofollow" onmousedown="this.href='javascript:';return true;" onclick="this.href='javascript:';return true;">rubyonrails-co...@googlegroups.com.
To post to this group, send email to <a href="javascript:" target="_blank" gdf-obfuscated-mailto="PGJjJIF-AgAJ" rel="nofollow" onmousedown="this.href='javascript:';return true;" onclick="this.href='javascript:';return true;">rubyonra...@googlegroups.com.
Visit this group at <a href="https://groups.google.com/group/rubyonrails-core" target="_blank" rel="nofollow" onmousedown="this.href='https://groups.google.com/group/rubyonrails-core';return true;" onclick="this.href='https://groups.google.com/group/rubyonrails-core';return true;">https://groups.google.com/group/rubyonrails-core.
For more options, visit <a href="https://groups.google.com/d/optout" target="_blank" rel="nofollow" onmousedown="this.href='https://groups.google.com/d/optout';return true;" onclick="this.href='https://groups.google.com/d/optout';return true;">https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: Magic number in tests

Artur Beljajev
OK, I am on it.

On Wednesday, February 21, 2018 at 9:47:06 AM UTC+2, Kasper Timm Hansen wrote:
Artur, send a pull request :)

--
Kasper

On 21 Feb 2018, 01.14 +0100, Artur Beljajev <<a href="javascript:" target="_blank" gdf-obfuscated-mailto="M-z6yAA2AQAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">artur.b...@...>, wrote:
Should I create a ticket on Github first?

On Saturday, January 20, 2018 at 3:28:49 PM UTC+2, Kasper Timm Hansen wrote:
Yep, that sounds good too, Kerri.

Artur, go right ahead!

--
Kasper

On 10 Jan 2018, 15.37 +0100, Kerri Miller <[hidden email]>, wrote:
Not to "well actually" myself but another option might be to use MAX_PASSWORD_LENGTH_ALLOWED as the OP suggested, but have an explicit test for MAX_PASSWORD_LENGTH_ALLOWED=72.. which might actually be a better approach, since it'll isolate the exact defect case (MAX_PASSWORD_LENGTH_ALLOWED != 72) rather than have it be implied by a well-named-but-oddly-disconnected constant.

// Kerri

On Wed, Jan 10, 2018 at 3:19 PM, <[hidden email]> wrote:
I would be happy to do this!


On Tuesday, January 9, 2018 at 10:30:41 PM UTC+2, Kasper Timm Hansen wrote:
Yep, we could do that 👍

Den 9. jan. 2018 kl. 12.39 skrev Kerri Miller <[hidden email]>:

I could see an argument made for making it a constant in the _test_ (EXPECTED_MAX_PASSWORD_LENGTH), which at least would document /why/ the number is being used.

On Tue, Jan 9, 2018 at 11:37 AM, Kasper Timm Hansen <[hidden email]> wrote:
I’m -1 on changing that. I think the constant 72 is used directly such that tests will fail if anybody changes MAX_PASSWORD_LENGTH_ALLOWED.

With your change in, a later change to MAX_PASSWORD_LENGTH_ALLOWED by some other contributor wouldn’t make any tests fail.

Since that constant isn’t meant to change without us thinking more about it, I’d say we keep the code as is.

Thanks for starting to contribute!

Den 9. jan. 2018 kl. 09.56 skrev [hidden email]:

Magic number 72 is used many times throughout the tests, which is actually present under ActiveModel::SecurePassword::MAX_PASSWORD_LENGTH_ALLOWED constant. I propose to replace that magic number either by the constant or method call.

I have never contributed to such large open source project as Rails. Please let me know your thoughts.
 


<a href="https://github.com/rails/rails/blob/master/activemodel/test/cases/secure_password_test.rb" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fblob%2Fmaster%2Factivemodel%2Ftest%2Fcases%2Fsecure_password_test.rb\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNGaLKGr9A14qSiG3WQraGuHgwAvMw&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fblob%2Fmaster%2Factivemodel%2Ftest%2Fcases%2Fsecure_password_test.rb\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNGaLKGr9A14qSiG3WQraGuHgwAvMw&#39;;return true;">https://github.com/rails/rails/blob/master/activemodel/test/cases/secure_password_test.rb
<a href="https://github.com/rails/rails/blob/ff657e73f0b61a7e224a9f158467ed2ec915bd9b/activemodel/lib/active_model/secure_password.rb" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fblob%2Fff657e73f0b61a7e224a9f158467ed2ec915bd9b%2Factivemodel%2Flib%2Factive_model%2Fsecure_password.rb\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEWQruGyMZXbRw6FLmz6Fk9ddtpyA&#39;;return true;" onclick="this.href=&#39;https://www.google.com/url?q\x3dhttps%3A%2F%2Fgithub.com%2Frails%2Frails%2Fblob%2Fff657e73f0b61a7e224a9f158467ed2ec915bd9b%2Factivemodel%2Flib%2Factive_model%2Fsecure_password.rb\x26sa\x3dD\x26sntz\x3d1\x26usg\x3dAFQjCNEWQruGyMZXbRw6FLmz6Fk9ddtpyA&#39;;return true;">https://github.com/rails/rails/blob/ff657e73f0b61a7e224a9f158467ed2ec915bd9b/activemodel/lib/active_model/secure_password.rb

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to [hidden email].
Visit this group at <a href="https://groups.google.com/group/rubyonrails-core" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;">https://groups.google.com/group/rubyonrails-core.
For more options, visit <a href="https://groups.google.com/d/optout" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;">https://groups.google.com/d/optout.

--
Kasper


--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to [hidden email].
Visit this group at <a href="https://groups.google.com/group/rubyonrails-core" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;">https://groups.google.com/group/rubyonrails-core.
For more options, visit <a href="https://groups.google.com/d/optout" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;">https://groups.google.com/d/optout.


--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to [hidden email].
Visit this group at <a href="https://groups.google.com/group/rubyonrails-core" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;">https://groups.google.com/group/rubyonrails-core.
For more options, visit <a href="https://groups.google.com/d/optout" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;">https://groups.google.com/d/optout.

--
Kasper

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to [hidden email].
Visit this group at <a href="https://groups.google.com/group/rubyonrails-core" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;">https://groups.google.com/group/rubyonrails-core.
For more options, visit <a href="https://groups.google.com/d/optout" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;">https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to [hidden email].
Visit this group at <a href="https://groups.google.com/group/rubyonrails-core" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;">https://groups.google.com/group/rubyonrails-core.
For more options, visit <a href="https://groups.google.com/d/optout" rel="nofollow" target="_blank" onmousedown="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;">https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to <a href="javascript:" target="_blank" gdf-obfuscated-mailto="M-z6yAA2AQAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">rubyonrails-co...@googlegroups.com.
To post to this group, send email to <a href="javascript:" target="_blank" gdf-obfuscated-mailto="M-z6yAA2AQAJ" rel="nofollow" onmousedown="this.href=&#39;javascript:&#39;;return true;" onclick="this.href=&#39;javascript:&#39;;return true;">rubyonra...@googlegroups.com.
Visit this group at <a href="https://groups.google.com/group/rubyonrails-core" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/group/rubyonrails-core&#39;;return true;">https://groups.google.com/group/rubyonrails-core.
For more options, visit <a href="https://groups.google.com/d/optout" target="_blank" rel="nofollow" onmousedown="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;" onclick="this.href=&#39;https://groups.google.com/d/optout&#39;;return true;">https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.