Re: [PATCH 08/16] test-reach: create new test tool for ref_newer

2018-07-18 Thread Derrick Stolee

On 7/16/2018 7:00 PM, Jonathan Tan wrote:

To use the new test-tool, use 'test-tool reach ' and provide
input to stdin that describes the inputs to the method. Currently, we
only implement the ref_newer method, which requires two commits. Use
lines "A:" and "B:" for the two inputs. We will
expand this input later to accommodate methods that take lists of
commits.

It would be nice if "A" and "B" were "ancestor" and "descendant" (or
something like that) instead, so that I don't have to check which
direction the reach is calculated in.


Different methods will use different combinations. I do notice that I 
forgot to list the method parameters as part of the test-tool output. It 
should print "ref_newer(A,B)" so you know which input goes in which place.





+int cmd__reach(int ac, const char **av)
+{
+   struct object_id oid_A, oid_B;
+   struct strbuf buf = STRBUF_INIT;
+   struct repository *r = the_repository;
+
+   setup_git_directory();
+
+   if (ac < 2)
+   exit(1);
+
+
+   while (strbuf_getline(, stdin) != EOF) {
+   struct object_id oid;
+   struct object *o;
+   struct commit *c;
+   if (buf.len < 3)
+   continue;
+
+   if (get_oid_committish(buf.buf + 2, ))
+   die("failed to resolve %s", buf.buf + 2);

You can also use skip_prefix() instead of using arithmetic to determine
the start of the OID.


+# Construct a grid-like commit graph with points (x,y)
+# with 1 <= x <= 10, 1 <= y <= 10, where (x,y) has
+# parents (x-1, y) and (x, y-1), keeping in mind that
+# we drop a parent if a coordinate is nonpositive.
+#
+# (10,10)
+#/   \
+# (10,9)(9,10)
+#/ \   /  \
+#(10,8)(9,9)  (8,10)
+#   / \/   \  /\
+# ( continued...)
+#   \ /\   /  \/
+#(3,1) (2,2)  (1,3)
+#\ /\ /
+# (2,1)  (2,1)
+#  \/
+#  (1,1)

This is quite a good design, thanks.


+# We use branch 'comit-x-y' to refer to (x,y).

s/comit/commit/


+   git show-ref -s commit-7-7 | git commit-graph write --stdin-commits &&
+   mv .git/objects/info/commit-graph commit-graph-half &&

My understanding is that this writes for 7-7 and all its ancestors,
but...


+test_expect_success 'ref_newer:hit' '
+   cat >input <<-\EOF &&
+   A:commit-5-7
+   B:commit-2-3
+   EOF
+   printf "ref_newer:1\n" >expect &&
+   test_three_modes ref_newer
+'
+
+test_done

...both 5-7 and 2-3 are ancestors of 7-7, right? Which means that you
don't test the "half" commit graph here. (It's probably sufficient to
just adjust the numbers.)


Good point! Thanks. I'll just write the commit-graph starting at 
commit-5-5 and that should satisfy the point of the "mixed-mode" tests.


-Stolee



Re: [PATCH 08/16] test-reach: create new test tool for ref_newer

2018-07-16 Thread Jonathan Tan
> To use the new test-tool, use 'test-tool reach ' and provide
> input to stdin that describes the inputs to the method. Currently, we
> only implement the ref_newer method, which requires two commits. Use
> lines "A:" and "B:" for the two inputs. We will
> expand this input later to accommodate methods that take lists of
> commits.

It would be nice if "A" and "B" were "ancestor" and "descendant" (or
something like that) instead, so that I don't have to check which
direction the reach is calculated in.

> +int cmd__reach(int ac, const char **av)
> +{
> + struct object_id oid_A, oid_B;
> + struct strbuf buf = STRBUF_INIT;
> + struct repository *r = the_repository;
> +
> + setup_git_directory();
> +
> + if (ac < 2)
> + exit(1);
> +
> +
> + while (strbuf_getline(, stdin) != EOF) {
> + struct object_id oid;
> + struct object *o;
> + struct commit *c;
> + if (buf.len < 3)
> + continue;
> +
> + if (get_oid_committish(buf.buf + 2, ))
> + die("failed to resolve %s", buf.buf + 2);

You can also use skip_prefix() instead of using arithmetic to determine
the start of the OID.

> +# Construct a grid-like commit graph with points (x,y)
> +# with 1 <= x <= 10, 1 <= y <= 10, where (x,y) has
> +# parents (x-1, y) and (x, y-1), keeping in mind that
> +# we drop a parent if a coordinate is nonpositive.
> +#
> +# (10,10)
> +#/   \
> +# (10,9)(9,10)
> +#/ \   /  \
> +#(10,8)(9,9)  (8,10)
> +#   / \/   \  /\
> +# ( continued...)
> +#   \ /\   /  \/
> +#(3,1) (2,2)  (1,3)
> +#\ /\ /
> +# (2,1)  (2,1)
> +#  \/
> +#  (1,1)

This is quite a good design, thanks.

> +# We use branch 'comit-x-y' to refer to (x,y).

s/comit/commit/

> + git show-ref -s commit-7-7 | git commit-graph write --stdin-commits &&
> + mv .git/objects/info/commit-graph commit-graph-half &&

My understanding is that this writes for 7-7 and all its ancestors,
but...

> +test_expect_success 'ref_newer:hit' '
> + cat >input <<-\EOF &&
> + A:commit-5-7
> + B:commit-2-3
> + EOF
> + printf "ref_newer:1\n" >expect &&
> + test_three_modes ref_newer
> +'
> +
> +test_done

...both 5-7 and 2-3 are ancestors of 7-7, right? Which means that you
don't test the "half" commit graph here. (It's probably sufficient to
just adjust the numbers.)