git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] Add tests for rev-list --graph with options that simplify history

From
ASAdam Simpkins <simpkins@facebook.com>
Date
Aug 21, 2009, 21:23 UTC
Message-ID
<20090821212326.GX8147@facebook.com>
In-Reply-To
<7vbpm8exeo.fsf@alter.siamese.dyndns.org>
On Fri, Aug 21, 2009 at 01:15:27PM -0700, Junio C Hamano wrote:
Show 9 quoted lines
> Adam Simpkins <simpkins@facebook.com> 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.

Show 6 quoted lines
> 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@facebook.com
Previous: Junio C HamanoNext: Santi Béjar
Message 13 of 14 in “RE: interaction between --graph and --simplify-by-decoration”
  1. Adam SimpkinsAug 18, 2009
  2. graph API: fix bug in graph_is_interesting()Adam Simpkins, Aug 18, 2009
  3. Junio C HamanoAug 18, 2009
  4. Adam SimpkinsAug 19, 2009
  5. Adam SimpkinsAug 19, 2009
  6. Junio C HamanoAug 19, 2009
  7. Junio C HamanoAug 19, 2009
  8. Adam SimpkinsAug 19, 2009
  9. Add test case for rev-list --parents --show-allAdam Simpkins, Aug 19, 2009
  10. Junio C HamanoAug 20, 2009
  11. Add tests for rev-list --graph with options that simplify historyAdam Simpkins, Aug 21, 2009
  12. Junio C HamanoAug 21, 2009
  13. Adam SimpkinsAug 21, 2009
  14. Santi BéjarAug 21, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.