Re: [PATCH v2 15/18] test-reach: test commit_contains

[Derrick Stolee]

On 7/25/2018 2:08 PM, Junio C Hamano wrote:

Jonathan Tan  writes:


+   } else if (!strcmp(av[1], "commit_contains")) {
+   struct ref_filter filter;
+   struct contains_cache cache;
+   init_contains_cache();
+
+   if (ac > 2 && !strcmp(av[2], "--tag"))
+   filter.with_commit_tag_algo = 1;
+   else
+   filter.with_commit_tag_algo = 0;
+
+   printf("%s(_,A,X,_):%d\n", av[1], commit_contains(, A, X, 
));

Should we initialize filter (with {NULL} or some equivalent)?

Sounds like a sensible suggestion.  Wouldn't we segfault otherwise
depending on what garbage bytes are on the stack?


It's a good idea to initialize the struct properly, but the only part of 
the 'filter' struct that is accessed by that method is the 
'with_commit_tag_algo' member. Everything else is read from A, X, and cache.


Thanks,
-Stolee

Re: [PATCH v2 15/18] test-reach: test commit_contains

[Junio C Hamano]

Jonathan Tan  writes:

>> +} else if (!strcmp(av[1], "commit_contains")) {
>> +struct ref_filter filter;
>> +struct contains_cache cache;
>> +init_contains_cache();
>> +
>> +if (ac > 2 && !strcmp(av[2], "--tag"))
>> +filter.with_commit_tag_algo = 1;
>> +else
>> +filter.with_commit_tag_algo = 0;
>> +
>> +printf("%s(_,A,X,_):%d\n", av[1], commit_contains(, A, 
>> X, ));
>
> Should we initialize filter (with {NULL} or some equivalent)?

Sounds like a sensible suggestion.  Wouldn't we segfault otherwise
depending on what garbage bytes are on the stack?

Re: [PATCH v2 15/18] test-reach: test commit_contains

[Jonathan Tan]

> + } else if (!strcmp(av[1], "commit_contains")) {
> + struct ref_filter filter;
> + struct contains_cache cache;
> + init_contains_cache();
> +
> + if (ac > 2 && !strcmp(av[2], "--tag"))
> + filter.with_commit_tag_algo = 1;
> + else
> + filter.with_commit_tag_algo = 0;
> +
> + printf("%s(_,A,X,_):%d\n", av[1], commit_contains(, A, 
> X, ));

Should we initialize filter (with {NULL} or some equivalent)?

[PATCH v2 15/18] test-reach: test commit_contains

[Derrick Stolee]

The commit_contains method has two modes which depend on the given
ref_filter struct. We have the "normal" algorithm (which is also the
typically-slow operation) and the "tag" algorithm. This difference is
essentially what changes performance for 'git branch --contains' versus
'git tag --contains'. There are thoughts that the data shapes used by
these two applications justify the different implementations.

Create tests using 'test-tool reach commit_contains [--tag]' to cover
both methods.

Signed-off-by: Derrick Stolee 
---
 t/helper/test-reach.c | 12 
 t/t6600-test-reach.sh | 34 ++
 2 files changed, 46 insertions(+)

diff --git a/t/helper/test-reach.c b/t/helper/test-reach.c
index c79729cac0..eb21103998 100644
--- a/t/helper/test-reach.c
+++ b/t/helper/test-reach.c
@@ -4,6 +4,7 @@
 #include "commit-reach.h"
 #include "config.h"
 #include "parse-options.h"
+#include "ref-filter.h"
 #include "string-list.h"
 #include "tag.h"
 
@@ -112,6 +113,17 @@ int cmd__reach(int ac, const char **av)
print_sorted_commit_ids(list);
} else if (!strcmp(av[1], "can_all_from_reach")) {
printf("%s(X,Y):%d\n", av[1], can_all_from_reach(X, Y, 1));
+   } else if (!strcmp(av[1], "commit_contains")) {
+   struct ref_filter filter;
+   struct contains_cache cache;
+   init_contains_cache();
+
+   if (ac > 2 && !strcmp(av[2], "--tag"))
+   filter.with_commit_tag_algo = 1;
+   else
+   filter.with_commit_tag_algo = 0;
+
+   printf("%s(_,A,X,_):%d\n", av[1], commit_contains(, A, 
X, ));
}
 
exit(0);
diff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh
index e41eb397a7..d139a00d1d 100755
--- a/t/t6600-test-reach.sh
+++ b/t/t6600-test-reach.sh
@@ -205,4 +205,38 @@ test_expect_success 'can_all_from_reach:miss' '
test_three_modes can_all_from_reach
 '
 
+test_expect_success 'commit_contains:hit' '
+   cat >input <<-\EOF &&
+   A:commit-7-7
+   X:commit-2-10
+   X:commit-3-9
+   X:commit-4-8
+   X:commit-5-7
+   X:commit-6-6
+   X:commit-7-5
+   X:commit-8-4
+   X:commit-9-3
+   EOF
+   echo "commit_contains(_,A,X,_):1" >expect &&
+   test_three_modes commit_contains &&
+   test_three_modes commit_contains --tag
+'
+
+test_expect_success 'commit_contains:miss' '
+   cat >input <<-\EOF &&
+   A:commit-6-5
+   X:commit-2-10
+   X:commit-3-9
+   X:commit-4-8
+   X:commit-5-7
+   X:commit-6-6
+   X:commit-7-5
+   X:commit-8-4
+   X:commit-9-3
+   EOF
+   echo "commit_contains(_,A,X,_):0" >expect &&
+   test_three_modes commit_contains &&
+   test_three_modes commit_contains --tag
+'
+
 test_done
-- 
2.18.0.118.gd4f65b8d14