Engines for Rails 2.0 test application

Rails Engines Development
1

Hi James, hi all,

Like we agreed on #engines last week I’ve worked on Engines for Rails
2.0
(still using RC1) this weekend. I’ve commited my work here:

http://svn.artweb-design.de/engines/tests/rails_2_0

Please criticize everything :slight_smile:

Analog to the test apps in the Engines repo this is a complete rails
app. You
should be able to run the tests once you’ve created the database(s).
Apperently the migration tests uses the development database at the
moment
when you use rake test:units. (I’ll have to figure out how to only
rely on the
test database, TODO)

Based on our discussion on IRC I’ve added tests and further refactored
the
Engines plugin.

As for the tests I had to invest some time to figure out how Al Pacino,
Baldwin, Stephen and William work together with James and how they all
cope
with funk, bananas and hobbits! (Which was quite fun.)

So I’ve decided to restructure this using some more descriptive
names …
hoping to increase comprehensibility a little. I hope this is ok for
you.
Also, doing this I think I was able to remove some duplicate and add
some
missing functionality to test.

I believe that the tests now cover the important parts (not sure though)
aside from these (TODO):

  • ActionMailer breaks with the Engines::RailsExtension (tests disabled)
  • $LOAD_PATH seems to be in reverse order so tests fail here (tests
    disabled)
  • no tests for arbitrary code mixing ported (unclear what’s going to
    drop)

As for the refactoring, I’ve changed quite a bit (mostly but not solely
responding to your considerations in IRC):

  • moved Rails.plugins to Engines.plugins
  • removed about.yml and version attribute
  • removed Engines.current
  • removed block from Engines::PluginList, left the rest there for now
  • removed LegacySupport
  • added an Engines::Assets module to hold file/dir creation/copying
    related
    stuff
  • added the following line in Engines::RailsExtensions::Routing
    map = self # allows to just copy and paste routes around
  • moved extensions to an accessible attribute on Engines and
    extensions loding
    from init.rb to Engines.init to declutter the init.rb file
  • supposedly quite some stuff that I forgot to take notes about …

Engines and Engines::Plugin are pretty lean now. I moved stuff out of
init.rb
because I thought it would be a nice idea to have the Engines plugin
behave
just like a normal Rails plugin if it hasn’t been “booted”. I think this
also responds to your consideration to mess as little with Rails core
code as
possible.

(On the other hand) I’ve added a default_attr_accessor method to Class
in
order to dry up the repetition in Engine::Plugin. Personally I feel
that this
extension to Class is quite useful (it could even go into
ActiveSupport). If
you don’t like it, I’ll revert this.

Regarding the location of the routes.rb file: I’m sure this has been
discussed
before, but as there are the plugin subdirectories /db/migrations and /
public
… why isn’t the directory structure mirrored for the routes file?
I.e. why
is it located in the plugin root dir instead of a /config subdir? IMHO
mirroring the directory structure here would make things even more
intuitive.

–
sven fuchs [email protected]
artweb design http://www.artweb-design.de
grünberger 65 + 49 (0) 30 - 47 98 69 96 (phone)
d-10245 berlin + 49 (0) 171 - 35 20 38 4 (mobile)

2

On Dec 2, 2007, at 8:06 AM, Sven F. wrote:

Analog to the test apps in the Engines repo this is a complete rails
Engines plugin.
you.

What’s the cause of the failure?

Cheers,
Pascal.

http://blog.nanorails.com

3

Woha, folks, I appologize for the awkward formatting of my last
mail … I shouldn’t have just pasted the text from Textmate :frowning:

Am 02.12.2007 um 21:04 schrieb Pascal:

On Dec 2, 2007, at 8:06 AM, Sven F. wrote:

Not quite. I was aware that it’s there … but I haven’t been able to
really follow it. I’ll try to catch up on that.

What’s the cause of the failure?

I’m not sure, yet. It might be caused by my own changes to Engines,
some changes from Rails 1.2 to Rails 2.0 RC1 (which I used) or even a
bug in my testcases.

I’m not even totally sure if I really understand how things are
supposed to work here.

Can you give me a recap?

–
sven fuchs [email protected]
artweb design http://www.artweb-design.de
grünberger 65 + 49 (0) 30 - 47 98 69 96 (phone)
d-10245 berlin + 49 (0) 171 - 35 20 38 4 (mobile)

4

Thanks for all the pointers, Pascal.

After reading the thread I now think that the problem is simply in my
testcase as it’s based my former assumption that the plugin’s
load_paths should appear in the order in that they were loaded/
initialized.

Am 02.12.2007 um 21:45 schrieb Pascal:

disabled)
bug in my testcases.

macros/base’)

http://lists.rails-engines.org/listinfo.cgi/engine-developers-rails-engines.org
–
sven fuchs [email protected]
artweb design http://www.artweb-design.de
grünberger 65 + 49 (0) 30 - 47 98 69 96 (phone)
d-10245 berlin + 49 (0) 171 - 35 20 38 4 (mobile)

5

On Dec 2, 2007, at 12:17 PM, Sven F. wrote:

supposed to work here.

Can you give me a recap?
Comments 11 Pascal and 13 James A. should give you an idea of what’s
going on and possible issues with that. Basically, $LOAD_PATH is in
reverse order to give plugins initialized later more weight. The
problem is that if the path to a file is used more than once, the
first one gets totally hidden.

Possible solutions to ensure a plugin loads its code and nothing else
in the load path:

  • use absolute paths init.rb to require all the files
    require File.join(File.dirname(FILE), ‘lib/filtered_column/
    processor’)
    require File.join(File.dirname(FILE), ‘lib/filtered_column/mixin’)
    require File.join(File.dirname(FILE), ‘lib/filtered_column/filters/
    base’)
    require File.join(File.dirname(FILE), ‘lib/filtered_column/macros/
    base’)

  • insert your lib in front of everyon in init.rb
    $:.unshift(File.dirname(FILE) + ‘/lib’)

the second solution is easier, but can introduce a whole new set of
loading problems for other plugins since you are changing the load path.

Cheers,
Pascal.

http://blog.nanorails.com

6

Hi James,

I’ve commited everything to the Engines repo now:
http://svn.rails-engines.org/test/engines/rails_2.0/

Thanks for granting me commit access!

Am 02.12.2007 um 23:16 schrieb James A.:

I believe the rendering mechanism for ActionMailer has changed, I’ll
investigate this.

Great.

See Pascal’s comments about the $LOAD_PATH - I
believe it is in the correct order. I’m not sure that your test for
this makes sense, but I’ll double check this.

Yes, I’m pretty sure my test is just plain crap. I’ll change that.

While code mixing might be deprecated, it could be worth keeping parts
of it around - for instance, I’m quite keen on moving the explicit
declaration of “controllers and helpers” out of the depths of the
Dependencies extensions to somewhere more visible.

Aha, ok. Well, I’ve put porting the tests for that on my todo list.

  • moved Rails.plugins to Engines.plugins
    This could be problematic, as existing migrations will refer to
    Rails.plugins. I suggest that we add a method to the Rails module
    which returns the result of a call to Engines.plugins.

Ok.

possible.
I’m not sure if I follow you here - by ‘not booted’, presumably you
mean that the line hasn’t been added to the top of environment.rb?

Yes. “booted” because it happens right after Rails’ own boot.rb has
finished … i.e. just “present” or “included”.

I might be wrong but I think previously you’d had to remove the entire
Engines plugin from the plugins directory to deactivate it. Or comment
out everything in the init.rb file? I’m not sure if it’s actually a
big win to be able to deactivate it by commenting out the line in
environment but it feels a little cleaner (ignoring the fact that it
doesn’t really work that way when there are map.from_route mappings in
routes.rb).

I sympathise, but I think it’s possibly wise to avoid messing around
with Ruby core classes unless it’s unavoidable. I’d be more
comfortable setting the defaults in a more explicit way.

Ok, I’ll revert that.

The distinction here is that the only file in our “config” directory
would be the routes.rb file. Is it really worth creating a directory
to hold that single file? Personally, I don’t think so. I also feel
that it’s worth breaking away from the “your plugin is a mini
application” mindset that mirroring the default app layout so
completely would promote.

Hmm, I guess that’s a matter of taste probably. Ok, strike that.

Anyway - I need to spend some time merging what you’ve done into my
own refactorings, and then I’m sure I’ll have more to add. Thanks for
your work so far Sven!

It’s a lot of fun, actually. And I’m happy to be able to contribute to
such a great project. :slight_smile:

–
sven fuchs [email protected]
artweb design http://www.artweb-design.de
grünberger 65 + 49 (0) 30 - 47 98 69 96 (phone)
d-10245 berlin + 49 (0) 171 - 35 20 38 4 (mobile)

7

James,

I’ve committed my work again on Monday.

Like I’ve mentioned previously I probably won’t have that much time
to work on Engines until the weekend, probably even Wednesday next week.

Besides the ActionMailer there are two things that I couldn’t get
working without further investigation:

Arbitrary code mixing: the test on line 34 in
http://dev.rails-engines.org/repository/file/test/engines/rails_2.0/test/unit/arbitrary_code_mixing_test.rb

Model/Lib loading precedence:
http://dev.rails-engines.org/repository/file/test/engines/rails_2.0/test/unit/model_and_lib_test.rb
(ignore the note about functional/view_loading_test.rb in there)

I’ve got the load_paths test working though:
http://dev.rails-engines.org/repository/file/test/engines/rails_2.0/test/unit/load_path_test.rb

Am 03.12.2007 um 01:15 schrieb Sven F.:

Great.

declaration of “controllers and helpers” out of the depths of the

core code as
big win to be able to deactivate it by commenting out the line in
The distinction here is that the only file in our “config” directory
your work so far Sven!
d-10245 berlin + 49 (0) 171 - 35 20 38 4 (mobile)

Engine-Developers mailing list
[email protected]
http://lists.rails-engines.org/listinfo.cgi/engine-developers-rails-engines.org

–
sven fuchs [email protected]
artweb design http://www.artweb-design.de
grünberger 65 + 49 (0) 30 - 47 98 69 96 (phone)
d-10245 berlin + 49 (0) 171 - 35 20 38 4 (mobile)

8

On Dec 2, 2007 4:06 PM, Sven F. [email protected] wrote:

Hi James, hi all,

Hi Sven,

Like we agreed on #engines last week I’ve worked on Engines for Rails 2.0
(still using RC1) this weekend. I’ve commited my work here:

http://svn.artweb-design.de/engines/tests/rails_2_0

I’ve checked out your code, and I’ll be looking over it this week :slight_smile:

As for the tests I had to invest some time to figure out how Al Pacino,
Baldwin, Stephen and William work together with James and how they all
cope with funk, bananas and hobbits! (Which was quite fun.)

So I’ve decided to restructure this using some more descriptive names …
hoping to increase comprehensibility a little. I hope this is ok for you.

That seems fair :slight_smile:

  • ActionMailer breaks with the Engines::RailsExtension (tests disabled)
  • $LOAD_PATH seems to be in reverse order so tests fail here (tests
    disabled)
  • no tests for arbitrary code mixing ported (unclear what’s going to
    drop)

I believe the rendering mechanism for ActionMailer has changed, I’ll
investigate this. See Pascal’s comments about the $LOAD_PATH - I
believe it is in the correct order. I’m not sure that your test for
this makes sense, but I’ll double check this.

While code mixing might be deprecated, it could be worth keeping parts
of it around - for instance, I’m quite keen on moving the explicit
declaration of “controllers and helpers” out of the depths of the
Dependencies extensions to somewhere more visible.

As for the refactoring, I’ve changed quite a bit (mostly but not solely
responding to your considerations in IRC):

  • moved Rails.plugins to Engines.plugins

This could be problematic, as existing migrations will refer to
Rails.plugins. I suggest that we add a method to the Rails module
which returns the result of a call to Engines.plugins.

  • removed about.yml and version attribute
  • removed Engines.current
  • removed block from Engines::PluginList, left the rest there for now
  • removed LegacySupport
  • added an Engines::Assets module to hold file/dir creation/copying
    related stuff
  • added the following line in Engines::RailsExtensions::Routing
    map = self # allows to just copy and paste routes around

These all seem fine to me.

  • moved extensions to an accessible attribute on Engines and
    extensions loding from init.rb to Engines.init to declutter the init.rb file.
    Engines and Engines::Plugin are pretty lean now. I moved stuff out of
    init.rb because I thought it would be a nice idea to have the Engines plugin
    behave just like a normal Rails plugin if it hasn’t been “booted”. I think this
    also responds to your consideration to mess as little with Rails core code as
    possible.

I’m not sure if I follow you here - by ‘not booted’, presumably you
mean that the line hasn’t been added to the top of environment.rb?

(On the other hand) I’ve added a default_attr_accessor method to Class
in order to dry up the repetition in Engine::Plugin. Personally I feel
that this extension to Class is quite useful (it could even go into
ActiveSupport). If you don’t like it, I’ll revert this.

I sympathise, but I think it’s possibly wise to avoid messing around
with Ruby core classes unless it’s unavoidable. I’d be more
comfortable setting the defaults in a more explicit way.

Regarding the location of the routes.rb file:
… why isn’t the directory structure mirrored for the routes file?
I.e. why is it located in the plugin root dir instead of a /config subdir? IMHO
mirroring the directory structure here would make things even more
intuitive.

The distinction here is that the only file in our “config” directory
would be the routes.rb file. Is it really worth creating a directory
to hold that single file? Personally, I don’t think so. I also feel
that it’s worth breaking away from the “your plugin is a mini
application” mindset that mirroring the default app layout so
completely would promote.

Anyway - I need to spend some time merging what you’ve done into my
own refactorings, and then I’m sure I’ll have more to add. Thanks for
your work so far Sven!

Let me know if you have any other ideas,