Forum: Rails Engines development Engines for Rails 2.0 test application

402602a60e500e85f2f5dc1ff3648ecb?d=identicon&s=25 Sven Fuchs (Guest)
on 2007-12-02 17:07
(Received via mailing list)
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 :)

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      svenfuchs@artweb-design.de
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)
615c3ad6684a2831d34bbc8b5d1d2015?d=identicon&s=25 Pascal (Guest)
on 2007-12-02 21:04
(Received via mailing list)
On Dec 2, 2007, at 8:06 AM, Sven Fuchs wrote:

> Analog to the test apps in the Engines repo this is a complete rails
> Engines plugin.
> you.
> * $LOAD_PATH seems to be in reverse order so tests fail here (tests
> disabled)
Did you follow this discussion on
http://groups.google.com/group/rubyonrails-core/br...
?

What's the cause of the failure?

Cheers,
Pascal.
--
http://blog.nanorails.com
402602a60e500e85f2f5dc1ff3648ecb?d=identicon&s=25 Sven Fuchs (Guest)
on 2007-12-02 21:17
(Received via mailing list)
Woha, folks, I appologize for the awkward formatting of my last
mail ... I shouldn't have just pasted the text from Textmate :(

Am 02.12.2007 um 21:04 schrieb Pascal:
> On Dec 2, 2007, at 8:06 AM, Sven Fuchs wrote:
>>
>> * $LOAD_PATH seems to be in reverse order so tests fail here (tests
>> disabled)
> Did you follow this discussion on 
http://groups.google.com/group/rubyonrails-core/br...
> ?

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      svenfuchs@artweb-design.de
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)
615c3ad6684a2831d34bbc8b5d1d2015?d=identicon&s=25 Pascal (Guest)
on 2007-12-02 21:46
(Received via mailing list)
On Dec 2, 2007, at 12:17 PM, Sven Fuchs wrote:

>
> supposed to work here.
>
> Can you give me a recap?
Comments 11 Pascal and 13 James Adam 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
402602a60e500e85f2f5dc1ff3648ecb?d=identicon&s=25 Sven Fuchs (Guest)
on 2007-12-02 22:53
(Received via mailing list)
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...
--
sven fuchs      svenfuchs@artweb-design.de
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)
05d703f649ef1d07e78d7b479fb4c4ac?d=identicon&s=25 James Adam (Guest)
on 2007-12-02 23:17
(Received via mailing list)
On Dec 2, 2007 4:06 PM, Sven Fuchs <svenfuchs@artweb-design.de> 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 :)


> 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 :)


> * 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,
402602a60e500e85f2f5dc1ff3648ecb?d=identicon&s=25 Sven Fuchs (Guest)
on 2007-12-03 01:17
(Received via mailing list)
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 Adam:
> 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. :)



--
sven fuchs      svenfuchs@artweb-design.de
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)
402602a60e500e85f2f5dc1ff3648ecb?d=identicon&s=25 Sven Fuchs (Guest)
on 2007-12-05 10:51
(Received via mailing list)
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/...

Model/Lib loading precedence:
http://dev.rails-engines.org/repository/file/test/...
(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/...



Am 03.12.2007 um 01:15 schrieb Sven Fuchs:

> 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
> Engine-Developers@lists.rails-engines.org
> http://lists.rails-engines.org/listinfo.cgi/engine...

--
sven fuchs      svenfuchs@artweb-design.de
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)
This topic is locked and can not be replied to.