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.)


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

2018-07-16 Thread Derrick Stolee via GitGitGadget
From: Derrick Stolee 

As we prepare to change the behavior of the algorithms in
commit-reach.c, create a new test-tool subcommand 'reach' to test these
methods on interesting commit-graph shapes.

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.

The test t6600-test-reach.sh creates a repo whose commits form a
two-dimensional grid. This grid makes it easy for us to determine
reachability because commit-A-B can reach commit-X-Y if and only if A is
at least X and B is at least Y. This helps create interesting test cases
for each result of the methods in commit-reach.c.

We test all methods in three different states of the commit-graph file:
Non-existent (no generation numbers), fully computed, and mixed (some
commits have generation numbers and others do not).

Signed-off-by: Derrick Stolee 
---
 Makefile  |  1 +
 t/helper/test-reach.c | 62 +++
 t/helper/test-tool.c  |  1 +
 t/helper/test-tool.h  |  1 +
 t/t6600-test-reach.sh | 86 +++
 5 files changed, 151 insertions(+)
 create mode 100644 t/helper/test-reach.c
 create mode 100755 t/t6600-test-reach.sh

diff --git a/Makefile b/Makefile
index 59781f4bc..d69f9d415 100644
--- a/Makefile
+++ b/Makefile
@@ -716,6 +716,7 @@ TEST_BUILTINS_OBJS += test-mktemp.o
 TEST_BUILTINS_OBJS += test-online-cpus.o
 TEST_BUILTINS_OBJS += test-path-utils.o
 TEST_BUILTINS_OBJS += test-prio-queue.o
+TEST_BUILTINS_OBJS += test-reach.o
 TEST_BUILTINS_OBJS += test-read-cache.o
 TEST_BUILTINS_OBJS += test-ref-store.o
 TEST_BUILTINS_OBJS += test-regex.o
diff --git a/t/helper/test-reach.c b/t/helper/test-reach.c
new file mode 100644
index 0..8cc570f3b
--- /dev/null
+++ b/t/helper/test-reach.c
@@ -0,0 +1,62 @@
+#include "test-tool.h"
+#include "cache.h"
+#include "commit-reach.h"
+#include "config.h"
+#include "parse-options.h"
+#include "tag.h"
+
+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);
+
+   o = parse_object(r, );
+   o = deref_tag_noverify(o);
+
+   if (!o)
+   die("failed to load commit for input %s resulting in 
oid %s\n",
+   buf.buf, oid_to_hex());
+
+   c = object_as_type(r, o, OBJ_COMMIT, 0);
+
+   if (!c)
+   die("failed to load commit for input %s resulting in 
oid %s\n",
+   buf.buf, oid_to_hex());
+
+   switch (buf.buf[0]) {
+   case 'A':
+   oidcpy(_A, );
+   break;
+
+   case 'B':
+   oidcpy(_B, );
+   break;
+
+   default:
+   die("unexpected start of line: %c", buf.buf[0]);
+   }
+   }
+   strbuf_release();
+
+   if (!strcmp(av[1], "ref_newer"))
+   printf("%s:%d\n", av[1], ref_newer(_A, _B));
+
+   exit(0);
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index dafc91c24..582d02adf 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -26,6 +26,7 @@ static struct test_cmd cmds[] = {
{ "online-cpus", cmd__online_cpus },
{ "path-utils", cmd__path_utils },
{ "prio-queue", cmd__prio_queue },
+   { "reach", cmd__reach },
{ "read-cache", cmd__read_cache },
{ "ref-store", cmd__ref_store },
{ "regex", cmd__regex },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index 80cbcf085..a7e53c420 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -20,6 +20,7 @@ int cmd__mktemp(int argc, const char **argv);
 int cmd__online_cpus(int argc, const char **argv);
 int cmd__path_utils(int argc, const char **argv);
 int cmd__prio_queue(int argc, const char **argv);
+int cmd__reach(int argc, const char **argv);
 int cmd__read_cache(int argc, const char **argv);
 int cmd__ref_store(int argc, const char **argv);
 int cmd__regex(int argc, const char **argv);
diff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh
new file mode 100755
index 0..4ffe0174d
--- /dev/null
+++ b/t/t6600-test-reach.sh
@@