I''d like to get to work patching ActionPack, but there''s an unfortunate annoyance stopping me for now. I''ve described it here: http://dev.rubyonrails.org/ticket/8849 The gist of it is that render_test.rb and new_render_test.rb define conflicting types for Customer, and thus effect each other. I haven''t fixed this myself because I''d like to know the intent here, and what the proper solution is. Is render_test.rb just old code that should be thrown away or merged into new_render_test.rb? Or should we instead focus on encapsulating their individual definitions of Customer? I''d like someone to look at this before I go proposing the changes. --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
On Tue, Jul 03, 2007 at 05:46:00PM -0000, Nick wrote:> I''d like to get to work patching ActionPack, but there''s an > unfortunate annoyance stopping me for now. > > I''ve described it here: http://dev.rubyonrails.org/ticket/8849 > > The gist of it is that render_test.rb and new_render_test.rb define > conflicting types for Customer, and thus effect each other. I haven''tThis sounds an awful lot like http://dev.rubyonrails.org/ticket/8714. - Matt -- When the revolution comes, they won''t be able to FIND the wall. -- Brian Kantor, in the Monastery --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
Oh goodie, so this is being taken care of after all? I hope my redundant report has drawn some more attention to this. I mean, with the emphasis on test-based development, the tests should be rather important, yes? I hope this patch, or a better fix, is adopted soon! On Jul 3, 2:38 pm, Matthew Palmer <mpal...@hezmatt.org> wrote:> On Tue, Jul 03, 2007 at 05:46:00PM -0000, Nick wrote: > > I''d like to get to work patching ActionPack, but there''s an > > unfortunate annoyance stopping me for now. > > > I''ve described it here:http://dev.rubyonrails.org/ticket/8849 > > > The gist of it is that render_test.rb and new_render_test.rb define > > conflicting types for Customer, and thus effect each other. I haven''t > > This sounds an awful lot likehttp://dev.rubyonrails.org/ticket/8714. > > - Matt > > -- > When the revolution comes, they won''t be able to FIND the wall. > -- Brian Kantor, in the Monastery--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
> > This sounds an awful lot likehttp://dev.rubyonrails.org/ticket/8714.Seems it should be relatively straightforward to extract the definition to a file which is required by the tests which need it? I''d be happy to apply a patch which did that. As for the more hypothetical problem of tests messing with each other and causing problems. Any patches which fix real problems caused by that will be gladly applied. -- Cheers Koz --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
Argh, this isn''t the only problem. Try checking for namespace intrusions in the other tests. Things like "Module Fun" sneak in and modify other things. This whole testing suite needs some serious refactoring. I don''t think rake''s tests were ever meant to have anything outside of the test class in the same file. Looking at the way things are arranged, there''s a lot of incompatible duplication. Also, I noticed there''s a controller_fixtures directory that is not even being used. Wouldn''t this be the prime place to put examples of controllers that would be used in the tests, instead of defining those controllers in the tests themselves? On Jul 3, 4:12 pm, "Michael Koziarski" <mich...@koziarski.com> wrote:> > > This sounds an awful lot likehttp://dev.rubyonrails.org/ticket/8714. > > Seems it should be relatively straightforward to extract the > definition to a file which is required by the tests which need it? > I''d be happy to apply a patch which did that. > > As for the more hypothetical problem of tests messing with each other > and causing problems. Any patches which fix real problems caused by > that will be gladly applied. > > -- > Cheers > > Koz--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
> Argh, this isn''t the only problem. Try checking for namespace > intrusions in the other tests. Things like "Module Fun" sneak in and > modify other things. This whole testing suite needs some serious > refactoring. I don''t think rake''s tests were ever meant to have > anything outside of the test class in the same file. > > Looking at the way things are arranged, there''s a lot of incompatible > duplication. Also, I noticed there''s a controller_fixtures directory > that is not even being used. Wouldn''t this be the prime place to put > examples of controllers that would be used in the tests, instead of > defining those controllers in the tests themselves?What are the practical implications of these issues that you''re describing? I guess I''m just not following you? The tests could certainly be broken up to have all the controllers defined in seperate files. However they''re still going be run in one interpreter. -- Cheers Koz --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
On Wed, Jul 04, 2007 at 11:12:59AM +1200, Michael Koziarski wrote:> > > This sounds an awful lot likehttp://dev.rubyonrails.org/ticket/8714. > > Seems it should be relatively straightforward to extract the > definition to a file which is required by the tests which need it? > I''d be happy to apply a patch which did that.Sold. I''ll work up a patch that meets with your approval. Any particular place you would suggest that file should go? I''m really unfamiliar with the Rails codebase, so I don''t know where a good place for this file would be. The best I can find through a quick trawl is actionpack/test/controller/controller_fixtures/app/models, but there''s no directory there currently and it seems a bit presumptuous to be creating new directories with my first real patch. <grin> If you can let me know where the best place to put this pseudo-model is, I''ll get the patch sorted out ASAP. - Matt --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
> What are the practical implications of these issues that you''re > describing? I guess I''m just not following you?We''re talking about how some test files are defining identifiers in the global namespace which are affecting each other, possibly causing different results when the tests are run alone or in a different order. In this case it''s just repeated definitions which need to be placed elsewhere. But, it seems like there should be a hard and fast rule in testing against placing anything mutable in the global namespace, lest another test reference it and get different results.> The tests could certainly be broken up to have all the controllers > defined in seperate files. However they''re still going be run in one > interpreter.Yes, but as long as they''re all using the same controller, rather than using different ones that could clobber each other differently based on test order and cause undefined results, I''d count it as an improvement. But truly, finding a way to purge this information between tests would be quite beneficial!> The best I can find through a quick trawl is > actionpack/test/controller/controller_fixtures/app/models, but there''s no > directory there currently and it seems a bit presumptuous to be creating new > directories with my first real patch. <grin>Hehe, go ahead. I''ve been meaning to tear into this source tree, though I''m a little timid about submitting monster patch files that might break other people''s pending patches. What would be really nice would be for someone with cvs write access to take care of this, after skimming through the ticket tracker and applying any relevant actionpack/test-related patches. --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
> The best I can find through a quick trawl is > actionpack/test/controller/controller_fixtures/app/modelsHow about creating a file in the same directory where render_test.rb is located (i.e. actionpack/test/controller)? There''s a file there fake_controllers.rb that''s used by 4 test files: assert_select_test.rb, routing_test.rb, selector_test.rb, test_test.rb. Seems to me like this is just a similar case of refactoring (so maybe create a fake_models.rb?). This is just a suggestion though as I''m still new at this and still trying to make my first patch. What do you guys think? On Jul 4, 4:24 pm, Matthew Palmer <mpal...@hezmatt.org> wrote:> On Wed, Jul 04, 2007 at 11:12:59AM +1200, Michael Koziarski wrote: > > > > This sounds an awful lot likehttp://dev.rubyonrails.org/ticket/8714. > > > Seems it should be relatively straightforward to extract the > > definition to a file which is required by the tests which need it? > > I''d be happy to apply a patch which did that. > > Sold. I''ll work up a patch that meets with your approval. Any particular > place you would suggest that file should go? I''m really unfamiliar with the > Rails codebase, so I don''t know where a good place for this file would be. > The best I can find through a quick trawl is > actionpack/test/controller/controller_fixtures/app/models, but there''s no > directory there currently and it seems a bit presumptuous to be creating new > directories with my first real patch. <grin> > > If you can let me know where the best place to put this pseudo-model is, > I''ll get the patch sorted out ASAP. > > - Matt--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
On Fri, Jul 06, 2007 at 02:58:20PM -0000, mikong wrote:> > > The best I can find through a quick trawl is > > actionpack/test/controller/controller_fixtures/app/models > > How about creating a file in the same directory where render_test.rb > is located (i.e. actionpack/test/controller)? There''s a file there > fake_controllers.rb that''s used by 4 test files: > assert_select_test.rb, routing_test.rb, selector_test.rb, > test_test.rb. Seems to me like this is just a similar case of > refactoring (so maybe create a fake_models.rb?). This is just a > suggestion though as I''m still new at this and still trying to make my > first patch. What do you guys think?I hadn''t noticed the fake_controllers.rb, since I usually don''t put anything but *_test.rb in my tests directory. That looks like a good precedent, though, so I''ll rework my patch in that direction and Koz can tell me if it''s bodgy. <grin> - Matt -- "The user-friendly computer is a red herring. The user-friendliness of a book just makes it easier to turn pages. There''s nothing user-friendly about learning to read." -- Alan Kay --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
On Jul 4, 7:12 am, "Michael Koziarski" <mich...@koziarski.com> wrote:> > > This sounds an awful lot likehttp://dev.rubyonrails.org/ticket/8714. > > Seems it should be relatively straightforward to extract the > definition to a file which is required by the tests which need it? > I''d be happy to apply a patch which did that.Koz, a patch has been made for ticket #8714 that extracted the definition to a file as described by Matt in the recent posts. Can you please check it and comment if you still find any problems with it? --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---