{"thread":{"id":"48582","subject":"[RFC PATCH v4] Implement --first-parent for git rev-list --bisect","startedAt":"2018-05-28T09:20:46Z","lastAt":"2018-05-28T21:51:49Z","messageCount":5,"participants":["Tiago Botelho","Junio C Hamano"],"isPatch":true,"patchVersion":4,"patchTotal":null},"messages":[{"id":"348650","messageId":"20180528092017.19022-1-tiagonbotelho@hotmail.com","threadId":"48582","inReplyTo":null,"subject":"[RFC PATCH v4] Implement --first-parent for git rev-list --bisect","fromName":"Tiago Botelho","fromEmail":"tiagonbotelho@gmail.com","sentAt":"2018-05-28T09:20:17Z","receivedAt":"2018-05-28T09:20:46Z","isPatch":true,"sender":{"key":"tiagonbotelho@gmail.com","avatar":"https://gravatar.com/avatar/263ea75fe0d4aa4ac14fa9dd08b82955689b6769cf5d33599914ca4321a4597b?d=mp&s=160"},"body":"This will enable users to implement bisecting on first parents\nwhich can be useful for when the commits from a feature branch\nthat we want to merge are not always tested.\n\nSigned-off-by: Tiago Botelho <tiagonbotelho@hotmail.com>\n---\n\nThis patch adds all Junio's suggestions, namely do_find_bisection() being\nbroken when assigning q's weight to p if in first parent mode and q being\nnot UNINTERESTING and its weight still not being known.\n\nThe graph displayed in the unit tests was also changed from being top-bottom\nto be left-right in order to keep it consistent with graphs in other tests.\n\n bisect.c                   | 45 ++++++++++++++++++++++++++++++---------------\n bisect.h                   |  3 ++-\n builtin/rev-list.c         |  3 +++\n revision.c                 |  3 ---\n t/t6002-rev-list-bisect.sh | 37 +++++++++++++++++++++++++++++++++++++\n 5 files changed, 72 insertions(+), 19 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 4eafc8262..e58cb8d62 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -33,6 +33,8 @@ static const char *term_good;\n  *\n  * We care just barely enough to avoid recursing for\n  * non-merge entries.\n+ *\n+ * Note: This function does not support the usage --first-parent.\n  */\n static int count_distance(struct commit_list *entry)\n {\n@@ -82,15 +84,16 @@ static inline void weight_set(struct commit_list *elem, int weight)\n \t*((int*)(elem->item->util)) = weight;\n }\n \n-static int count_interesting_parents(struct commit *commit)\n+static int count_interesting_parents(struct commit *commit, unsigned bisect_flags)\n {\n \tstruct commit_list *p;\n \tint count;\n \n \tfor (count = 0, p = commit->parents; p; p = p->next) {\n-\t\tif (p->item->object.flags & UNINTERESTING)\n-\t\t\tcontinue;\n-\t\tcount++;\n+\t\tif (!(p->item->object.flags & UNINTERESTING))\n+\t\t    count++;\n+\t\tif (bisect_flags & BISECT_FIRST_PARENT)\n+\t\t\tbreak;\n \t}\n \treturn count;\n }\n@@ -117,10 +120,10 @@ static inline int halfway(struct commit_list *p, int nr)\n }\n \n #if !DEBUG_BISECT\n-#define show_list(a,b,c,d) do { ; } while (0)\n+#define show_list(a,b,c,d,e) do { ; } while (0)\n #else\n static void show_list(const char *debug, int counted, int nr,\n-\t\t      struct commit_list *list)\n+\t\t      struct commit_list *list, unsigned bisect_flags)\n {\n \tstruct commit_list *p;\n \n@@ -146,10 +149,14 @@ static void show_list(const char *debug, int counted, int nr,\n \t\telse\n \t\t\tfprintf(stderr, \"---\");\n \t\tfprintf(stderr, \" %.*s\", 8, oid_to_hex(&commit->object.oid));\n-\t\tfor (pp = commit->parents; pp; pp = pp->next)\n+\t\tfor (pp = commit->parents; pp; pp = pp->next) {\n \t\t\tfprintf(stderr, \" %.*s\", 8,\n \t\t\t\toid_to_hex(&pp->item->object.oid));\n \n+\t\t\tif (bisect_flags & BISECT_FIRST_PARENT)\n+\t\t\t\tbreak;\n+\t\t}\n+\n \t\tsubject_len = find_commit_subject(buf, &subject_start);\n \t\tif (subject_len)\n \t\t\tfprintf(stderr, \" %.*s\", subject_len, subject_start);\n@@ -267,13 +274,13 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\tunsigned flags = commit->object.flags;\n \n \t\tp->item->util = &weights[n++];\n-\t\tswitch (count_interesting_parents(commit)) {\n+\t\tswitch (count_interesting_parents(commit, bisect_flags)) {\n \t\tcase 0:\n \t\t\tif (!(flags & TREESAME)) {\n \t\t\t\tweight_set(p, 1);\n \t\t\t\tcounted++;\n \t\t\t\tshow_list(\"bisection 2 count one\",\n-\t\t\t\t\t  counted, nr, list);\n+\t\t\t\t\t  counted, nr, list, bisect_flags);\n \t\t\t}\n \t\t\t/*\n \t\t\t * otherwise, it is known not to reach any\n@@ -289,7 +296,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t}\n \t}\n \n-\tshow_list(\"bisection 2 initialize\", counted, nr, list);\n+\tshow_list(\"bisection 2 initialize\", counted, nr, list, bisect_flags);\n \n \t/*\n \t * If you have only one parent in the resulting set\n@@ -319,7 +326,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\tcounted++;\n \t}\n \n-\tshow_list(\"bisection 2 count_distance\", counted, nr, list);\n+\tshow_list(\"bisection 2 count_distance\", counted, nr, list, bisect_flags);\n \n \twhile (counted < nr) {\n \t\tfor (p = list; p; p = p->next) {\n@@ -329,6 +336,11 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t\tif (0 <= weight(p))\n \t\t\t\tcontinue;\n \t\t\tfor (q = p->item->parents; q; q = q->next) {\n+\t\t\t\tif ((bisect_flags & BISECT_FIRST_PARENT)) {\n+\t\t\t\t\tif (weight(q) < 0)\n+\t\t\t\t\t\tq = NULL;\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n \t\t\t\tif (q->item->object.flags & UNINTERESTING)\n \t\t\t\t\tcontinue;\n \t\t\t\tif (0 <= weight(q))\n@@ -346,7 +358,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t\t\tweight_set(p, weight(q)+1);\n \t\t\t\tcounted++;\n \t\t\t\tshow_list(\"bisection 2 count one\",\n-\t\t\t\t\t  counted, nr, list);\n+\t\t\t\t\t  counted, nr, list, bisect_flags);\n \t\t\t}\n \t\t\telse\n \t\t\t\tweight_set(p, weight(q));\n@@ -357,7 +369,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,\n \t\t}\n \t}\n \n-\tshow_list(\"bisection 2 counted all\", counted, nr, list);\n+\tshow_list(\"bisection 2 counted all\", counted, nr, list, bisect_flags);\n \n \tif (!find_all)\n \t\treturn best_bisection(list, nr);\n@@ -372,7 +384,7 @@ void find_bisection(struct commit_list **commit_list, int *reaches,\n \tstruct commit_list *list, *p, *best, *next, *last;\n \tint *weights;\n \n-\tshow_list(\"bisection 2 entry\", 0, 0, *commit_list);\n+\tshow_list(\"bisection 2 entry\", 0, 0, *commit_list, bisect_flags);\n \n \t/*\n \t * Count the number of total and tree-changing items on the\n@@ -395,7 +407,7 @@ void find_bisection(struct commit_list **commit_list, int *reaches,\n \t\ton_list++;\n \t}\n \tlist = last;\n-\tshow_list(\"bisection 2 sorted\", 0, nr, list);\n+\tshow_list(\"bisection 2 sorted\", 0, nr, list, bisect_flags);\n \n \t*all = nr;\n \tweights = xcalloc(on_list, sizeof(*weights));\n@@ -962,6 +974,9 @@ int bisect_next_all(const char *prefix, int no_checkout)\n \tif (skipped_revs.nr)\n \t\tbisect_flags |= BISECT_FIND_ALL;\n \n+\tif (revs.first_parent_only)\n+\t\tbisect_flags |= BISECT_FIRST_PARENT;\n+\n \tfind_bisection(&revs.commits, &reaches, &all, bisect_flags);\n \trevs.commits = managed_skipped(revs.commits, &tried);\n \ndiff --git a/bisect.h b/bisect.h\nindex 1d40a33ad..dfb7faeec 100644\n--- a/bisect.h\n+++ b/bisect.h\n@@ -1,7 +1,8 @@\n #ifndef BISECT_H\n #define BISECT_H\n \n-#define BISECT_FIND_ALL\t\t(1u<<0)\n+#define BISECT_FIND_ALL \t(1u<<0)\n+#define BISECT_FIRST_PARENT \t(1u<<1)\n \n /*\n  * Find bisection. If something is found, `reaches` will be the number of\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex 8752f5bbe..66439e1b3 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -538,6 +538,9 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \tif (bisect_list) {\n \t\tint reaches, all;\n \n+\t\tif (revs.first_parent_only)\n+\t\t\tbisect_flags |= BISECT_FIRST_PARENT;\n+\n \t\tfind_bisection(&revs.commits, &reaches, &all, bisect_flags);\n \n \t\tif (bisect_show_vars)\ndiff --git a/revision.c b/revision.c\nindex 4e0e193e5..b9d063805 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2474,9 +2474,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \tif (!revs->reflog_info && revs->grep_filter.use_reflog_filter)\n \t\tdie(\"cannot use --grep-reflog without --walk-reflogs\");\n \n-\tif (revs->first_parent_only && revs->bisect)\n-\t\tdie(_(\"--first-parent is incompatible with --bisect\"));\n-\n \tif (revs->expand_tabs_in_log < 0)\n \t\trevs->expand_tabs_in_log = revs->expand_tabs_in_log_default;\n \ndiff --git a/t/t6002-rev-list-bisect.sh b/t/t6002-rev-list-bisect.sh\nindex a66140803..774d9a4fd 100755\n--- a/t/t6002-rev-list-bisect.sh\n+++ b/t/t6002-rev-list-bisect.sh\n@@ -263,4 +263,41 @@ test_expect_success 'rev-parse --bisect can default to good/bad refs' '\n \ttest_cmp expect.sorted actual.sorted\n '\n \n+# We generate the following commit graph:\n+#\n+#   B - C\n+#  /      \\\n+# A        FX\n+#  \\      /\n+#   D - EX\n+\n+test_expect_success 'setup' '\n+  test_commit A &&\n+  test_commit B &&\n+  test_commit C &&\n+  git reset --hard A &&\n+  test_commit D &&\n+  test_commit EX &&\n+  test_merge FX C\n+'\n+\n+test_output_expect_success \"--bisect --first-parent\" 'git rev-list --bisect --first-parent FX ^A' <<EOF\n+$(git rev-parse EX)\n+EOF\n+\n+test_output_expect_success \"--bisect-vars --first-parent\" 'git rev-list --bisect-vars --first-parent FX ^A' <<EOF\n+bisect_rev='$(git rev-parse EX)'\n+bisect_nr=1\n+bisect_good=0\n+bisect_bad=1\n+bisect_all=3\n+bisect_steps=1\n+EOF\n+\n+test_output_expect_success \"--bisect-all --first-parent\" 'git rev-list --bisect-all --first-parent FX ^A' <<EOF\n+$(git rev-parse EX) (dist=1)\n+$(git rev-parse D) (dist=1)\n+$(git rev-parse FX) (dist=0)\n+EOF\n+\n test_done\n-- \n2.16.3\n\n"},{"id":"348667","messageId":"xmqqzi0jj4jc.fsf@gitster-ct.c.googlers.com","threadId":"48582","inReplyTo":"20180528092017.19022-1-tiagonbotelho@hotmail.com","subject":"Re: [RFC PATCH v4] Implement --first-parent for git rev-list --bisect","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-05-28T13:25:27Z","receivedAt":"2018-05-28T13:25:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tiago Botelho <tiagonbotelho@gmail.com> writes:\n\n> diff --git a/bisect.c b/bisect.c\n> index 4eafc8262..e58cb8d62 100644\n> --- a/bisect.c\n> +++ b/bisect.c\n> @@ -33,6 +33,8 @@ static const char *term_good;\n>   *\n>   * We care just barely enough to avoid recursing for\n>   * non-merge entries.\n> + *\n> + * Note: This function does not support the usage --first-parent.\n>   */\n\nHmph, is this because we know --first-parent codepath currently does\nnot call this function, so we do not bother to prepare this function\nto be called from --first-parent codepath?\n\nI am not saying that we must prepare this function to be callable\nwith --first-parent; if I have to wonder why the above comment is\nthere and what it is trying to say, I suspect most other readers\nwould, too, so...\n\n> diff --git a/t/t6002-rev-list-bisect.sh b/t/t6002-rev-list-bisect.sh\n> index a66140803..774d9a4fd 100755\n> --- a/t/t6002-rev-list-bisect.sh\n> +++ b/t/t6002-rev-list-bisect.sh\n> @@ -263,4 +263,41 @@ test_expect_success 'rev-parse --bisect can default to good/bad refs' '\n>  \ttest_cmp expect.sorted actual.sorted\n>  '\n>  \n> +# We generate the following commit graph:\n> +#\n> +#   B - C\n> +#  /      \\\n> +# A        FX\n> +#  \\      /\n> +#   D - EX\n> +\n> +test_expect_success 'setup' '\n> +  test_commit A &&\n> +  test_commit B &&\n> +  test_commit C &&\n> +  git reset --hard A &&\n> +  test_commit D &&\n> +  test_commit EX &&\n> +  test_merge FX C\n> +'\n> +\n> +test_output_expect_success \"--bisect --first-parent\" 'git rev-list --bisect --first-parent FX ^A' <<EOF\n> +$(git rev-parse EX)\n> +EOF\n> +\n> +test_output_expect_success \"--bisect-vars --first-parent\" 'git rev-list --bisect-vars --first-parent FX ^A' <<EOF\n> +bisect_rev='$(git rev-parse EX)'\n> +bisect_nr=1\n> +bisect_good=0\n> +bisect_bad=1\n> +bisect_all=3\n> +bisect_steps=1\n> +EOF\n> +\n> +test_output_expect_success \"--bisect-all --first-parent\" 'git rev-list --bisect-all --first-parent FX ^A' <<EOF\n> +$(git rev-parse EX) (dist=1)\n> +$(git rev-parse D) (dist=1)\n> +$(git rev-parse FX) (dist=0)\n> +EOF\n> +\n\nThese are all good basic tests, but can you come up with a test that\ndemonstrates breakage in the previous round that has been fixed in\nthis version of the patch?\n\nThanks.\n\n"},{"id":"348673","messageId":"xmqqpo1fj1qv.fsf@gitster-ct.c.googlers.com","threadId":"48582","inReplyTo":"20180528092017.19022-1-tiagonbotelho@hotmail.com","subject":"Re: [RFC PATCH v4] Implement --first-parent for git rev-list --bisect","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-05-28T14:25:44Z","receivedAt":"2018-05-28T14:26:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tiago Botelho <tiagonbotelho@gmail.com> writes:\n\n> This will enable users to implement bisecting on first parents\n> which can be useful for when the commits from a feature branch\n> that we want to merge are not always tested.\n>\n> Signed-off-by: Tiago Botelho <tiagonbotelho@hotmail.com>\n> ---\n>\n> This patch adds all Junio's suggestions, namely do_find_bisection() being\n> broken when assigning q's weight to p if in first parent mode and q being\n> not UNINTERESTING and its weight still not being known.\n>\n> The graph displayed in the unit tests was also changed from being top-bottom\n> to be left-right in order to keep it consistent with graphs in other tests.\n>\n>  bisect.c                   | 45 ++++++++++++++++++++++++++++++---------------\n>  bisect.h                   |  3 ++-\n>  builtin/rev-list.c         |  3 +++\n>  revision.c                 |  3 ---\n>  t/t6002-rev-list-bisect.sh | 37 +++++++++++++++++++++++++++++++++++++\n>  5 files changed, 72 insertions(+), 19 deletions(-)\n\n> diff --git a/t/t6002-rev-list-bisect.sh b/t/t6002-rev-list-bisect.sh\n> index a66140803..774d9a4fd 100755\n> --- a/t/t6002-rev-list-bisect.sh\n> +++ b/t/t6002-rev-list-bisect.sh\n> @@ -263,4 +263,41 @@ test_expect_success 'rev-parse --bisect can default to good/bad refs' '\n> ...\n> +test_output_expect_success \"--bisect-all --first-parent\" 'git rev-list --bisect-all --first-parent FX ^A' <<EOF\n> +$(git rev-parse EX) (dist=1)\n> +$(git rev-parse D) (dist=1)\n> +$(git rev-parse FX) (dist=0)\n> +EOF\n> +\n>  test_done\n\nRunning this test number of times gives me spurious errors.  Is the\norder of these output lines unstable?  How do we \"sort\" these\nbisect-all results?  If we are not sorting and the output depends on\nhappenstance, then probably we would need to compare the expected\nand actual output after sorting.  Or if the output depends on\nsomething under our control (e.g. they are related to topology and\nrelative commit timestamp), we probably should try to control that\n\"something\" tighter so that we can rely on the order of the lines in\nthe \"expect\" file.\n\nIt also appears that we have \"--bisect and --first-parent do not\nwork well together\" in t6000, which also needs to be updated.  I\nneeded the following squashed into this patch to make \"make test\"\npass.\n\ndiff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh\nindex 969e4e9e52..981198ae6e 100755\n--- a/t/t6000-rev-list-misc.sh\n+++ b/t/t6000-rev-list-misc.sh\n@@ -96,8 +96,8 @@ test_expect_success 'rev-list can show index objects' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success '--bisect and --first-parent can not be combined' '\n-\ttest_must_fail git rev-list --bisect --first-parent HEAD\n+test_expect_success '--bisect and --first-parent can now be combined' '\n+\tgit rev-list --bisect --first-parent HEAD\n '\n \n test_expect_success '--header shows a NUL after each commit' '\n"},{"id":"348676","messageId":"CAADF+x2q6HwW3BaLt76ZVikyu8Rz0Ykbf7iD=dBORigaONmtDA@mail.gmail.com","threadId":"48582","inReplyTo":"xmqqpo1fj1qv.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH v4] Implement --first-parent for git rev-list --bisect","fromName":"Tiago Botelho","fromEmail":"tiagonbotelho@gmail.com","sentAt":"2018-05-28T16:49:47Z","receivedAt":"2018-05-28T16:50:17Z","isPatch":true,"sender":{"key":"tiagonbotelho@gmail.com","avatar":"https://gravatar.com/avatar/263ea75fe0d4aa4ac14fa9dd08b82955689b6769cf5d33599914ca4321a4597b?d=mp&s=160"},"body":"On 28 May 2018 at 15:25, Junio C Hamano <gitster@pobox.com> wrote:\n\n> Tiago Botelho <tiagonbotelho@gmail.com> writes:\n\n> > This will enable users to implement bisecting on first parents\n> > which can be useful for when the commits from a feature branch\n> > that we want to merge are not always tested.\n> >\n> > Signed-off-by: Tiago Botelho <tiagonbotelho@hotmail.com>\n> > ---\n> >\n> > This patch adds all Junio's suggestions, namely do_find_bisection()\nbeing\n> > broken when assigning q's weight to p if in first parent mode and q\nbeing\n> > not UNINTERESTING and its weight still not being known.\n> >\n> > The graph displayed in the unit tests was also changed from being\ntop-bottom\n> > to be left-right in order to keep it consistent with graphs in other\ntests.\n> >\n> >  bisect.c                   | 45\n++++++++++++++++++++++++++++++---------------\n> >  bisect.h                   |  3 ++-\n> >  builtin/rev-list.c         |  3 +++\n> >  revision.c                 |  3 ---\n> >  t/t6002-rev-list-bisect.sh | 37 +++++++++++++++++++++++++++++++++++++\n> >  5 files changed, 72 insertions(+), 19 deletions(-)\n\n> > diff --git a/t/t6002-rev-list-bisect.sh b/t/t6002-rev-list-bisect.sh\n> > index a66140803..774d9a4fd 100755\n> > --- a/t/t6002-rev-list-bisect.sh\n> > +++ b/t/t6002-rev-list-bisect.sh\n> > @@ -263,4 +263,41 @@ test_expect_success 'rev-parse --bisect can\ndefault to good/bad refs' '\n> > ...\n> > +test_output_expect_success \"--bisect-all --first-parent\" 'git rev-list\n--bisect-all --first-parent FX ^A' <<EOF\n> > +$(git rev-parse EX) (dist=1)\n> > +$(git rev-parse D) (dist=1)\n> > +$(git rev-parse FX) (dist=0)\n> > +EOF\n> > +\n> >  test_done\n\n> Running this test number of times gives me spurious errors.  Is the\n> order of these output lines unstable?  How do we \"sort\" these\n> bisect-all results?  If we are not sorting and the output depends on\n> happenstance, then probably we would need to compare the expected\n> and actual output after sorting.  Or if the output depends on\n> something under our control (e.g. they are related to topology and\n> relative commit timestamp), we probably should try to control that\n> \"something\" tighter so that we can rely on the order of the lines in\n> the \"expect\" file.\n\nThe reason why the tests were failing was because the above \"old\" tests\ndid not make use of test_commit which in turn would make the sha of each\ncommit be different and as a result give unexpected outputs at times.\nIf I move them to the top of that file the tests will pass every time,\nwould that\nbe ok?\n\n> It also appears that we have \"--bisect and --first-parent do not\n> work well together\" in t6000, which also needs to be updated.  I\n> needed the following squashed into this patch to make \"make test\"\n> pass.\n\n> diff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh\n> index 969e4e9e52..981198ae6e 100755\n> --- a/t/t6000-rev-list-misc.sh\n> +++ b/t/t6000-rev-list-misc.sh\n> @@ -96,8 +96,8 @@ test_expect_success 'rev-list can show index objects' '\n>          test_cmp expect actual\n>   '\n\n> -test_expect_success '--bisect and --first-parent can not be combined' '\n> -       test_must_fail git rev-list --bisect --first-parent HEAD\n> +test_expect_success '--bisect and --first-parent can now be combined' '\n> +       git rev-list --bisect --first-parent HEAD\n>   '\n\n>   test_expect_success '--header shows a NUL after each commit' '\n"},{"id":"348685","messageId":"xmqq4lirih3k.fsf@gitster-ct.c.googlers.com","threadId":"48582","inReplyTo":"CAADF+x2q6HwW3BaLt76ZVikyu8Rz0Ykbf7iD=dBORigaONmtDA@mail.gmail.com","subject":"Re: [RFC PATCH v4] Implement --first-parent for git rev-list --bisect","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-05-28T21:51:43Z","receivedAt":"2018-05-28T21:51:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tiago Botelho <tiagonbotelho@gmail.com> writes:\n\n>> Running this test number of times gives me spurious errors.  Is the\n>> order of these output lines unstable?  How do we \"sort\" these\n>> bisect-all results?  If we are not sorting and the output depends on\n>> happenstance, then probably we would need to compare the expected\n>> and actual output after sorting.  Or if the output depends on\n>> something under our control (e.g. they are related to topology and\n>> relative commit timestamp), we probably should try to control that\n>> \"something\" tighter so that we can rely on the order of the lines in\n>> the \"expect\" file.\n>\n> The reason why the tests were failing was because the above \"old\" tests\n> did not make use of test_commit which in turn would make the sha of each\n> commit be different and as a result give unexpected outputs at times.\n> If I move them to the top of that file the tests will pass every time,\n> would that\n> be ok?\n\nThat means if somebody else needs to add other tests before the\nlocation you are planning to move your tests, your tests would\nsuddenly start failing, no?  That does not sound like a particularly\nrobust solution to me.\n"}]}