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

remove duplicated number_helper tests in AP. They are already in AS. #9457

Merged

Conversation

senny
Copy link
Member

@senny senny commented Feb 27, 2013

With 155cd5e the number_helpers were moved into AS all the tests were copied over
but the tests in AP were not deleted. This is confusing.

I removed all duplicated tests and reorganized the tests in AP to only test the
functionality, that is added in AP.

@senny
Copy link
Member Author

senny commented Feb 27, 2013

@carlosantoniodasilva @rafaelfranca could you take a look?

assert_equal("(800) 555-1212", number_to_phone(8005551212, {:area_code => true}))
assert_equal("", number_to_phone("", {:area_code => true}))
assert_equal("800 555 1212", number_to_phone(8005551212, {:delimiter => " "}))
assert_equal(nil, number_to_phone(nil))

Choose a reason for hiding this comment

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

This can be the 1st assertion.

@carlosantoniodasilva
Copy link
Member

I agree there's some sort of duplication between both, they were left in AP as a safety check I guess since the originals wehere there (and is where most people will use them from). Anyway, we can kill some no problem, just make sure to leave a handful for each helper :)

With 155cd5e the number_helpers were moved into AS all the tests were copied over
but the tests in AP were not deleted. This is confusing.

I removed all duplicated tests and reorganized the tests in AP to only test the
functionality, that is added in AP.
* use 1.9 style hash syntax
* don't use brances on assert_equal
* prefere " over '
carlosantoniodasilva added a commit that referenced this pull request Feb 27, 2013
…ests

Remove duplicated number_helper tests in AP. They are already in AS.

With 155cd5e the number_helpers were moved into AS all the tests were copied over but the tests in AP were not deleted. This is confusing.

This removes all duplicated tests and reorganized the tests in AP to only test the functionality, that is added in AP.

Also cleanup some of the number helper tests.
@carlosantoniodasilva carlosantoniodasilva merged commit ab87689 into rails:master Feb 27, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants