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

Show real LoadError on helpers require #10642

Merged
merged 1 commit into from
Jul 10, 2013
Merged

Show real LoadError on helpers require #10642

merged 1 commit into from
Jul 10, 2013

Conversation

LTe
Copy link
Contributor

@LTe LTe commented May 16, 2013

When helper try to require missing file rails will throw exception about
missing helper.

# app/helpers/my_helper.rb
require 'missing'

module MyHelper
end

And when we try do load helper

class ApplicationController
  helper :my
end

Rails will throw exception. This is wrong because there is a helper file.

Missing helper file helpers/my_helper.rb

Now when helper try to require non-existed file rails will throw proper exception.

No such file to load -- missing


class InvalidHelpersTest < ActiveSupport::TestCase
def test_controller_raise_error_about_real_require_problem
assert_raise(LoadError, "No such file to load -- very_invalid_file_name") do
Copy link
Member

Choose a reason for hiding this comment

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

does this really verify the error message? As far as I know this only verifies the type of exception see the minitest source

to verify the message you can use:

 e = assert_raise(LoadError) {}
 assert_equal "No such file to load -- very_invalid_file_name", e.message

@senny
Copy link
Member

senny commented Jun 15, 2013

I added a few comments, can you also add a short CHANGELOG entry?

@senny
Copy link
Member

senny commented Jul 4, 2013

@LTe ping, are you still on this PR?

@LTe
Copy link
Contributor Author

LTe commented Jul 5, 2013

@senny yes, I will create update soon :)

@LTe
Copy link
Contributor Author

LTe commented Jul 9, 2013

@senny updated

@senny
Copy link
Member

senny commented Jul 9, 2013

@LTe it does no longer apply, can you push a rebased version?

@@ -1,3 +1,7 @@
* Fix an issue where rails raise exception about missing helper where is should throw `LoadError`. When helper file exists and only loaded file from this helper does not exist rails should throw LoadError instead of MissingHelperError.
Copy link
Member

Choose a reason for hiding this comment

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

can you wrap this line to 80 chars?

@LTe
Copy link
Contributor Author

LTe commented Jul 9, 2013

@senny rebased and updated

When helper try to require missing file rails will throw exception about
missing helper.

  # app/helpers/my_helper.rb

  require 'missing'

  module MyHelper
  end

And when we try do load helper

  class ApplicationController
    helper :my
  end

Rails will throw exception. This is wrong because there is a helper
file.

  Missing helper file helpers/my_helper.rb

Now when helper try to require non-existed file rails will throw proper
exception.

  No such file to load -- missing
@senny
Copy link
Member

senny commented Jul 10, 2013

@LTe thanks for your contribution 💛

senny added a commit that referenced this pull request Jul 10, 2013
Show real LoadError on helpers require
@senny senny merged commit af4b654 into rails:master Jul 10, 2013
senny added a commit that referenced this pull request Aug 28, 2013
Show real LoadError on helpers require
Conflicts:
	actionpack/CHANGELOG.md

Closes #12055.
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.

None yet

2 participants