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

Removing unused setup code from Lotus::Loader test #17

Merged
merged 1 commit into from Jun 30, 2014

Conversation

lengarvey
Copy link
Contributor

The test fixture setup and @loader assignment aren't required in loader_test.rb in order for the test to pass. I suspect this is because the loading occurs on Lotus::Application.new but at some stage a different thing was planned.

This test is also strange because it claims to test Lotus::Loader#load! but never directly calls that method. It seems marginally related to the original OneLine application before #6 was merged where it looked like:

require 'lotus'

module OneFile
  class Application < Lotus::Application
    configure do
      routes do
        get '/', to: 'home#index'
      end
    end
  end
end

# OneFile::Application.new
# we could uncomment this to force framework duplication

module OneFile
  module Controllers
    module Home
      include OneFile::Controller

      action 'Index' do
        def call(params)
        end
      end
    end
  end

  module Views
    module Home
      class Index
        include OneFile::View

        def render
          'Hello'
        end
      end
    end
  end
end

This fails because at class declaration time the application has not been loaded and the framework hasn't been duped into the extra modules. Instead we could force this duplication to occur by running OneFile::Application.new before declaring our Controller and View modules but this then requires two instantiations of our application. Once here and once in config.ru

The test fixture setup and `@loader` assignment aren't required in `loader_test.rb` in order for the test to pass. I suspect this is because the loading occurs on `Lotus::Application.new` but at some stage a different thing was planned.

This test is also strange because it claims to test `Lotus::Loader#load!` but never directly calls that method. It seems marginally related to the original `OneLine` application before hanami#6 was merged where it looked like:

```ruby
require 'lotus'

module OneFile
  class Application < Lotus::Application
    configure do
      routes do
        get '/', to: 'home#index'
      end
    end
  end
end

# OneFile::Application.new
# we could uncomment this to force framework duplication

module OneFile
  module Controllers
    module Home
      include OneFile::Controller

      action 'Index' do
        def call(params)
        end
      end
    end
  end

  module Views
    module Home
      class Index
        include OneFile::View

        def render
          'Hello'
        end
      end
    end
  end
end
```

This fails because at class declaration time the application has not been loaded and the framework hasn't been duped into the extra modules. Instead we could force this duplication to occur by running `OneFile::Application.new` before declaring our `Controller` and `View` modules but this then requires two instantiations of our application. Once here and once in `config.ru`
@fuadsaud
Copy link

I still feel weird about this module/class duplication subject. Couldn't we solve this by injecting the application/configurations into the actions and views?

@jodosha
Copy link
Member

jodosha commented Jun 30, 2014

@fuadsaud What do you mean? Configuration is already injected into controllers/actions and views/layouts. The framework duplication is needed to allow two or more Lotus applications to live in the same Ruby process and to have independent configurations.

@jodosha
Copy link
Member

jodosha commented Jun 30, 2014

@lengarvey @fuadsaud BTW I think we should:

  1. Get rid of that example in README
  2. Evaluate the configuration when Lotus::Application.inherited is triggered, instead at the initialization time (if possible).

jodosha added a commit that referenced this pull request Jun 30, 2014
Removing unused setup code from Lotus::Loader test
@jodosha jodosha merged commit 43a5082 into hanami:master Jun 30, 2014
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

3 participants