Can someone take a quick lookat this patch and tell me if I''m crazy ? Alan --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
You''re crazy That patch doesn''t solve the issue. Hashes should not be compared by their string representations otherwise tests will always have incosistencies on different platforms/runs. The only course of action is to compare these two informal messages with hashes extracted and then compare hashes too by breaking them into key-value pairs and sorting. That requires a helper private method, or maybe such already exists? On 2/21/07, alancfrancis <alancfrancis@gmail.com> wrote:> > > Can someone take a quick lookat this patch and tell me if I''m crazy ? > > Alan--~--~---------~--~----~------------~-------~--~----~ 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 Feb 22, 9:08 am, "Mislav Marohnić" <mislav.maroh...@gmail.com> wrote:> You''re crazy > > That patch doesn''t solve the issue. Hashes should not be compared by their > string representations otherwise tests will always have incosistencies on > different platforms/runs.Thanks Misalv, Yup, I know theres a better way. What I was trying to figure out is if the test was, in fact, failing for everyone else too and if this patch fixed it (in a not very nice way). Is the test failing for everyone, or just me ? This is part of a lreger question, which I posted on earlier. As a new Rails contributer, when I check out Rails shoudl I expect all the tests in all the projects to work all the time ? If I get failures shoudl I assume that a) This is unusual. Either I''ve messed up somehiw, or there''s a broken build situation which is being worked on. b) This is commonplace. Dont worry about it unless I''m trying to work in that area. Thanks for listening. Alan --~--~---------~--~----~------------~-------~--~----~ 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 2/22/07, alancfrancis <alancfrancis@gmail.com> wrote:> > > Is the test failing for everyone, or just me ?It probably passes on Mac OS X, but fails on Linux. What platform do you test on? This isn''t the only inconsistency! Just recently I''ve submitted #7615 to deal with the issue of Dir not reading filenames from the filesystem in consistent order. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
I''m testing on MacOSX. So what we''re saying is it''s "understood" that a few tests may or may not fail depending on platform, and unless that''s what I''m directly seeking to fix, I should just ignore it for now (while I''m working on something different) and as long as the same tests fail when I''m done as failed when I started, thats ''okay'' (for some definition of okay). Fair summary of the "submit-a-patch" process ? A. On Feb 22, 11:52 am, "Mislav Marohnić" <mislav.maroh...@gmail.com> wrote:> On 2/22/07, alancfrancis <alancfran...@gmail.com> wrote: > > > > > Is the test failing for everyone, or just me ? > > It probably passes on Mac OS X, but fails on Linux. What platform do you > test on? > > This isn''t the only inconsistency! Just recently I''ve submitted #7615 to > deal with the issue of Dir not reading filenames from the filesystem in > consistent order.--~--~---------~--~----~------------~-------~--~----~ 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 2/22/07, alancfrancis <alancfrancis@gmail.com> wrote:> > > So what we''re saying is it''s "understood" that a few tests may or may > not fail depending on platform, and unless that''s what I''m directly > seeking to fix, I should just ignore it for now (while I''m working on > something different) and as long as the same tests fail when I''m done > as failed when I started, thats ''okay'' (for some definition of okay).Wow. I have no idea what you said in this sentence. What I''m saying is simple: some Rails unit tests will always be broken if they rely on consistent ordering when there is none (like with hashes). Although it''s not high priority (the codebase is not defected, just tests), eventually they should get fixed (but not with the #7614 approach). What I''m also saying is: don''t sweat about it. Concentrate on the realdefects (if there are any). --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
alexey.verkhovsky@gmail.com
2007-Feb-22 20:02 UTC
Re: patch - #7614 for failing actionpack test
On Feb 22, 2:08 am, "Mislav Marohnić" <mislav.maroh...@gmail.com> wrote:> Hashes should not be compared by their > string representations.No. But you can do this: assert_equal hash1.sort, hash2.sort As long as the keys have <=> method defined, it''ll work. Alex --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
> Wow. I have no idea what you said in this sentence. > > What I''m saying is simple: some Rails unit tests will always be broken if > they rely on consistent ordering when there is none (like with hashes).Wow. You seem to bein a bit of an asshole here. Not sure how I''ve offended you, but apologies if I have. Perhaps the reason you don''t have any idea what I''m saying is that unlike you, what I''m saying is *not* simple. I work on XP teams where TDD is our religion. No broken tests. Ever. A broken test and failing build is reason enough to stop the team working on anything else until the build is fixed. Whether it''s a simple typo, an incorrect test, or broken code. What I''m trying to ascertain is whether thats true here. Irrespective of that particular test and that partiular fix. The volume of testing and attention to detail on the rails project makes me imagine it is true, and that a failing test (any failing test) would be bad juju.> Although it''s not high priority (the codebase is not defected, just tests), > eventually they should get fixed (but not with the #7614 approach).You seem to suggest that it''s not true, and that a few broken tests are low priority. That may be true, but I have to say that your apparent inability to understand the question and the distinction means I don''t really hold your answer as particularly authoratative.> What I''m also saying is: don''t sweat about it. Concentrate on the > realdefects (if there are any).And again I suggest that in the teams I work with, failing tests* are* real defects. You obviously disagree. Fair enough. Let''s move on. Alan --~--~---------~--~----~------------~-------~--~----~ 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 Feb 22, 8:02 pm, "alexey.verkhov...@gmail.com" <alexey.verkhov...@gmail.com> wrote:> On Feb 22, 2:08 am, "Mislav Marohnić" <mislav.maroh...@gmail.com> > wrote: > > > Hashes should not be compared by their > > string representations. > > No. But you can do this: assert_equal hash1.sort, hash2.sort > As long as the keys have <=> method defined, it''ll work.Hi Alex, The test (and method) in question is about checking an error message which happens to contain a stringrep of a hash, so while what you''re suggesting is (of course) possible, it''s not immediately useful here :-) Effectively the test passes a hash into a method and checks that it gets a message something like "there was a problem with the hash {:key1 => value1, :key2 => :value2}". The way to fix it properly, I suspect, is for the method to sort the hash as it builds the string, so that the assert can just compare strings knowing that the hash will always be in a certain order in that string. Since I''m not terribly familiar with the codebase, and I was In "broken build! patch it quick!" mode, my quick''n''dirty patch just fixed the symptom, not the cause...all of which is teahcing me that broken build isn''t such a big emergency :-) Alan --~--~---------~--~----~------------~-------~--~----~ 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 2/22/07, Mislav Marohnić <mislav.marohnic@gmail.com> wrote:> On 2/22/07, alancfrancis <alancfrancis@gmail.com> wrote: > > > > So what we're saying is it's "understood" that a few tests may or may > > not fail depending on platform, and unless that's what I'm directly > > seeking to fix, I should just ignore it for now (while I'm working on > > something different) and as long as the same tests fail when I'm done > > as failed when I started, thats 'okay' (for some definition of okay). > > Wow. I have no idea what you said in this sentence. > > What I'm saying is simple: some Rails unit tests will always be broken if > they rely on consistent ordering when there is none (like with hashes). > Although it's not high priority (the codebase is not defected, just tests), > eventually they should get fixed (but not with the #7614 approach). > > What I'm also saying is: don't sweat about it. Concentrate on the real > defects (if there are any). > >What you're saying is "simple" but incorrect. I doubt anyone on the Rails core team would agree that broken tests aren't real defects or that fixing them isn't a high priority. Chad --~--~---------~--~----~------------~-------~--~----~ 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 Feb 23, 10:54 am, "alancfrancis" <alancfran...@gmail.com> wrote:> Wow. You seem to bein a bit of an asshole here. Not sure how I''ve > offended you, but apologies if I have.Sorry about that. It was uncalled for. Alan --~--~---------~--~----~------------~-------~--~----~ 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 2/23/07, Chad Fowler <chad@chadfowler.com> wrote:> > What you''re saying is "simple" but incorrect. > > I doubt anyone on the Rails core team would agree that broken tests > aren''t real defects or that fixing them isn''t a high priority.When I said "real defects" I meant defects in the codebase (I thought one could tell from the context). Tests can have defects, of course, as well as any other code. I''ve submitted a proper patch to tackle this specific issue: http://dev.rubyonrails.org/attachment/ticket/7614/hash-comparison.diff It modifies the test so that the hash string is extracted from the error message, broken into parts, sorted and compared. The actual error messages are compared without hashes in them. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
Mislav, First off, thanks for cleaning up after me. My Ruby wasn''t strong enough to quickly figure out how to do that (which is what I would have said was the right thing to do). Second, perhaps another thread, aren''t the tests part of the codebase ? Alan On Feb 23, 5:00 pm, "Mislav Marohnić" <mislav.maroh...@gmail.com> wrote:> On 2/23/07, Chad Fowler <c...@chadfowler.com> wrote: > > > > > What you''re saying is "simple" but incorrect. > > > I doubt anyone on the Rails core team would agree that broken tests > > aren''t real defects or that fixing them isn''t a high priority. > > When I said "real defects" I meant defects in the codebase (I thought one > could tell from the context). Tests can have defects, of course, as well as > any other code. I''ve submitted a proper patch to tackle this specific issue:http://dev.rubyonrails.org/attachment/ticket/7614/hash-comparison.diff > > It modifies the test so that the hash string is extracted from the error > message, broken into parts, sorted and compared. The actual error messages > are compared without hashes in them.--~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
> Second, perhaps another thread, aren''t the tests part of the > codebase ?We most certainly want all our tests passing all the time, however a defect in an assertion is less important, while still important, than a defect in the framework itself. So yes, you''re both right. -- 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 Feb 23, 10:46 pm, "Michael Koziarski" <mich...@koziarski.com> wrote:> So yes, you''re both right.How characteristically diplomatic. :-) I definitely have to reconsider that move to Auckland. A. --~--~---------~--~----~------------~-------~--~----~ 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 2/23/07, Mislav Marohnić <mislav.marohnic@gmail.com> wrote:> > > When I said "real defects" I meant defects in the codebase (I thought one > could tell from the context). Tests can have defects, of course, as well as > any other code. I've submitted a proper patch to tackle this specific > issue...Seems like David didn't see the ticket when he commited [6231]. That is too bad, because I really believe my patch (below) was more solid than breaking the check into 4 plain-string regular expressions. Oh, well ... -- http://dev.rubyonrails.org/changeset/6231 http://dev.rubyonrails.org/attachment/ticket/7614/hash-comparison.diff?format=txt --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---