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

WIP: Allow customising error pages #61

Closed
wants to merge 1 commit into from

Conversation

twe4ked
Copy link

@twe4ked twe4ked commented Jul 31, 2014

I long long time ago I said I'd work on this. Ended up being quite a bit different than I imagined and I had to ask quite a bit of poor @jodosha. I haven't had a chance yet to finish the tests (unpushed) but I thought I had better (finally) create a PR.

The issue I was running into with the tests was changing the working directory. It seems the working directory needs to be changed before the application is loaded or it won't be set to the correct location.

describe 'Error page test' do
  include Rack::Test::Methods

  before do
    @current_dir = Dir.pwd
    Dir.chdir FIXTURES_ROOT.join('one_file')
    require 'fixtures/one_file/application' # We have to require this after the `chdir`
    @app = OneFile::Application.new
  end

I was using a modified version of the one_file application. Any thoughts on the best way to handle this?

@twe4ked twe4ked mentioned this pull request Jul 31, 2014
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.21%) when pulling 51e1961 on twe4ked:custom-error-pages into 11ce2b3 on lotus:master.

@jodosha
Copy link
Member

jodosha commented Aug 4, 2014

@twe4ked Thanks for this PR, but it can't be merged without tests.
Would you be so kind to introduce them? Thank you.

@jodosha jodosha added the waiting label Aug 5, 2014
@jodosha
Copy link
Member

jodosha commented Aug 5, 2014

Closing in favor of #65

@jodosha jodosha closed this Aug 5, 2014
@twe4ked
Copy link
Author

twe4ked commented Aug 5, 2014

No worries :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants