On Fri, Aug 21, 2009 at 01:15:27PM -0700, Junio C Hamano wrote: > Adam Simpkins <simpkins@xxxxxxxxxxxx> writes: > > > +# There's more than one "correct" way to represent the history graphically. > > +# These tests depend on the current behavior of the graphing code. If the > > +# graphing code is ever changed to draw the output differently, these tests > > +# cases will need to be updated to know about the new layout. > > An ideal solution to such a problem would be not to write the tests that > way to require _the exact layout_ of the output. Yeah. In the past I've been hesitant to submit tests for the graph behavior for precisely this reason. However, having tests that check the exact layout seems better than not having tests at all. > What was the bug you were trying to fix? Was it that in a simplified > history some arcs are not connected whey they should be? It was an issue with a missing arc between two commits that should have been connected. In the past, other bugs (e.g., the one fixed in 2ecbd0a0) have caused arcs to appear connected to the wrong commit. > Can you test that without relying on other aspect (say, commits are marked > with '*' right now but a patch might change it to '^' for some commits) of > the output? > > I am just wondering how feasible it is the problem you are trying to > solve, not demanding you to solve it. In general, it seems like its not worthwhile trying to solve this problem. I don't expect changes to the graph layout to occur often. Modifying this test case if and when they do occur seems simpler and less error-prone than trying to write code that attempts to anticipate changes we might make in the future. When I wrote this comment, I was thinking more about potential changes in the way arcs are drawn in the output, or in the amount of padding. The problem you mentioned (accepting other characters other than '*' for commits) is easier, but I'm still not convinced we should try to solve it. For example, it's nice that the current code also tests that boundary commits are represented differently than non-boundary commits. Being too permissive in what we accept could also potentially hide bugs in the future. -- Adam Simpkins simpkins@xxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html