Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Mount Konacha::Engine to /konacha #25

Closed
wants to merge 2 commits into from

7 participants

Yury Korolev Jo Liss Jonas Nicklas Chris Smith John Firebaugh Matt Dreamer Chris Toomey
Yury Korolev

Very useful to test js with running instance. No need to start
rake konacha:serve

Just start normal dev server (rails s) and point your browser to
http://0.0.0.0:3000/konacha

Yury Korolev yury Mount Konacha::Engine to /konacha
Very useful to test js with running instance. No need to start
`rake konacha:serve`

Just start normal dev server (`rails s`) and point your browser to
`http://0.0.0.0:3000/konacha`
1f19d83
Jo Liss
Collaborator

[Pull request to have the Konacha in-browser tests mounted at localhost:3000/konacha instead of a separate localhost:3500/ server process.]

/cc @jfirebaugh
/cc @jnicklas (Evergreen) and @quartzmo (mocha_rails) -- do you guys have any insights if either of those approaches have any fundamental problems?

I like the idea of serving the Konacha specs from inside the development server process.

The way the routing is implemented in this patch though (automatically mapping the routes in the library code), it collides with catch-all routes in the app, for instance:

  get ':action', :controller => :play

It gives me "The action 'konacha' could not be found for PlayController". For it to work, I have to remove the catch-all route, or manually insert mount Konacha::Engine => "/konacha" in my routes.rb before that line.

mocha_rails uses a generator to insert the route at the top of routes.rb, which works with catch-all routes and is more explicit, but requires an extra installation step.

Thoughts?

config/routes.rb
@@ -1,4 +1,8 @@
Konacha::Engine.routes.draw do
- match "/" => "konacha/specs#specs"
- match "*path" => "konacha/specs#specs"
+ match "/" => "specs#specs"
+ match "*path" => "specs#specs"
+end
+
+Rails.application.routes.draw do
+ mount Konacha::Engine => "/konacha"
end
Jo Liss Collaborator
joliss added a note

This should be protected with unless Rails.env.production?, just in case the Konacha gem got loaded in production.

Yury Korolev
yury added a note
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Jonas Nicklas

In Evergreen we add the route from the Engine, which, yes, does cause problems with catch-all routes. I think it's the better solution though. Less cruft for the common case, less fragile.

Chris Smith

I think isolating the Konacha namespace is the right idea.

Jo Liss
Collaborator

@quartzmo You mean isolating as in having a separate server process (i.e. what Konacha is doing at the moment)?

Chris Smith

No sorry, I meant the line "isolate_namespace Konacha" which is also in this commit. I believe that's a good idea, I believe it requires the host app to declare the route, as I recall. I did it with mocha_rails. I don't believe Jasminerice does it.

John Firebaugh
Owner

So, the advantage of this is that you don't have to run a separate server? I'm not convinced.

I like the idea of isolation in a different sense -- that JS specs are isolated from the rest of your app. We want to be able access the app's asset pipeline, but not anything else. You do not want your JS specs making AJAX requests to the app, for example. That belongs in an integration test.

That, plus the downsides with regards to the catchall route or requiring additional routing configuration, make me -1 on this.

John Firebaugh
Owner

That said, I'm not opposed to the idea of making it possible to mount the Konacha engine inside your app, providing it's not the default and the existing default behavior continues to work.

Chris Smith

+1 for possible, but not default. With Jasmine rice I believe it's the default, and completely transparent, so that just by including the gem you get a /jasmine route. I do not like that personally.

Yury Korolev yury Prepend Konacha::Engine to Rails.application.routes
- Can't write specs for it, because of rspec/rspec-rails#469
- Simplify konacha routes.rb a little bit
173fabf
Yury Korolev

@joliss Just fixed catch-all routes issue.

@jfirebaugh Now it's quite easy to make mounting as an option. But what the main point to not making it default now? Run separate process is good way to isolate everything, but typing another command and hold another copy of your app in memory not a good point.

Sorry for my english.

John Firebaugh
Owner

Isolation is worth more to me than the small burden of running a separate command.

Yury Korolev

ok, closing then.

Yury Korolev yury closed this
John Firebaugh
Owner

Yury, I'd happily accept a PR to make mounting an option. I'm guessing that what you had here was not too far off from that?

Jo Liss
Collaborator

I think John's point about isolation is excellent.

That said, I'm not opposed to the idea of making it possible to mount the Konacha engine inside your app

I wonder if the ability to have Konacha run in the dev server process is worth the added complication. Maybe we should be opinionated and not even make this a choice. But I'm on the fence here. It's your call.

Yury Korolev

Agreed here. It way easier to add mount Konacha::Enging => /konacha on top of routes.rb instead of creating configuration file for konacha and set that option there. That is why, if this is not default then not worth it.

Matt Dreamer

@yury I agree it is very useful to test js with running instance
I tried adding mount Konacha::Engine => /konacha to routes.rb but when I navigate to localhost:3000/konacha the base page loads correctly then I get an error message like this for each of my tests:

Error: Failed to load test_spec.js.coffee.
Perhaps it failed to compile? Check the rake output for errors.
    at Context.<anonymous> (http://localhost:3000/assets/konacha/parent.js:11:17)
    at Test.require.register.Runnable.run (http://localhost:3000/assets/mocha.js:3716:32)
    at Runner.require.register.Runner.runTest (http://localhost:3000/assets/mocha.js:4069:10)
    at require.register.Runner.runTests.next (http://localhost:3000/assets/mocha.js:4115:12)
    at next (http://localhost:3000/assets/mocha.js:3995:14)
    at require.register.Runner.hooks (http://localhost:3000/assets/mocha.js:4004:7)
    at next (http://localhost:3000/assets/mocha.js:3952:23)
    at require.register.Runner.hook (http://localhost:3000/assets/mocha.js:3972:5)
    at process.removeListener.window.onerror (http://localhost:3000/assets/mocha.js:4921:44)

Cannot find how to fix this. Do you have any ideas?

Chris Toomey

I have just run into the same issue as @justmatt with the engine mounting.

Without mounting the engine, the rake konacha:run tests run properly and the page served by rake konacha:serve also runs the test properly.

If I mount the engine using the following line at the top of my config/routes.rb file:

mount Konacha::Engine, at: '/konacha' if Rails.env.development?
  • The rake task no longs works (failure msg included below)
  • The page served by rake konacha:serve no longer works (stack trace included below)

More confusing is that with the engine mounted, the test page works when served via Pow despite failing on the command line and the built in server.

Any pointers as to how to solve this issue?


Rake task failure msg:

> rake konacha:run                                                                                                                                                                                                                                                                                                    [1.9.3]
F

  Failed:  test_spec.js.coffee
    Error: Failed to load test_spec.js.coffee.
Perhaps it failed to compile? Check the rake output for errors.

Finished in 0.00 seconds
1 examples, 1 failed, 0 pending

Konacha serve page stack trace

Error: Failed to load test_spec.js.coffee.
Perhaps it failed to compile? Check the rake output for errors.
    at Context.<anonymous> (http://localhost:3500/assets/konacha/parent.js:11:17)
    at Test.Runnable.run (http://localhost:3500/assets/mocha.js:4047:32)
    at Runner.runTest (http://localhost:3500/assets/mocha.js:4404:10)
    at http://localhost:3500/assets/mocha.js:4450:12
    at next (http://localhost:3500/assets/mocha.js:4330:14)
    at http://localhost:3500/assets/mocha.js:4339:7
    at next (http://localhost:3500/assets/mocha.js:4287:23)
    at http://localhost:3500/assets/mocha.js:4307:5
    at process.removeListener.window.onerror (http://localhost:3500/assets/mocha.js:5259:44)
Chris Toomey

Just saw that the topic I outline above is covered in issue #118.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 4, 2012
  1. Yury Korolev

    Mount Konacha::Engine to /konacha

    yury authored
    Very useful to test js with running instance. No need to start
    `rake konacha:serve`
    
    Just start normal dev server (`rails s`) and point your browser to
    `http://0.0.0.0:3000/konacha`
  2. Yury Korolev

    Prepend Konacha::Engine to Rails.application.routes

    yury authored
    - Can't write specs for it, because of rspec/rspec-rails#469
    - Simplify konacha routes.rb a little bit
This page is out of date. Refresh to see the latest.
Showing with 17 additions and 3 deletions.
  1. +2 −3 config/routes.rb
  2. +5 −0 lib/konacha.rb
  3. +10 −0 lib/konacha/engine.rb
5 config/routes.rb
View
@@ -1,4 +1,3 @@
Konacha::Engine.routes.draw do
- match "/" => "konacha/specs#specs"
- match "*path" => "konacha/specs#specs"
-end
+ match "(*path)" => "specs#specs"
+end
5 lib/konacha.rb
View
@@ -6,6 +6,11 @@ module Konacha
class << self
attr_accessor :mode
+ def mode
+ # defaults to :server
+ @mode ||= :server
+ end
+
def serve
puts "Your tests are here:"
puts " http://localhost:#{port}/"
10 lib/konacha/engine.rb
View
@@ -2,6 +2,8 @@ module Konacha
class Engine < Rails::Engine
config.konacha = ActiveSupport::OrderedOptions.new
+ isolate_namespace Konacha
+
def self.application(app)
Rack::Builder.app do
map app.config.assets.prefix do
@@ -28,5 +30,13 @@ def self.application(app)
app.config.assets.paths << app.root.join(options.spec_dir).to_s
end
+
+ initializer "konacha.engine.mount" do
+ config.after_initialize do
+ ::Rails.application.routes.prepend do
+ mount ::Konacha::Engine => "/konacha"
+ end
+ end
+ end
end
end
Something went wrong with that request. Please try again.