Thread (53 messages) 53 messages, 6 authors, 2017-05-30

Re: [PATCH 08/29] grep: add tests for --threads=N and grep.threads

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2017-05-11 19:23:12

On Thu, May 11, 2017 at 8:36 PM, Brandon Williams [off-list ref] wrote:
On 05/11, Ævar Arnfjörð Bjarmason wrote:
quoted
Add tests for --threads=N being supplied on the command-line, or when
grep.threads=N being supplied in the configuration.

When the threading support was made run-time configurable in commit
89f09dd34e ("grep: add --threads=<num> option and grep.threads
configuration", 2015-12-15) no tests were added for it.

In developing a change to the grep code I was able to make
'--threads=1 <pat>` segfault, while the test suite still passed. This
change fixes that blind spot in the tests.

In addition to asserting that asking for N threads shouldn't segfault,
test that the grep output given any N is the same.

The choice to test only 1..10 as opposed to 1..8 or 1..16 or whatever
is arbitrary. Testing 1..1024 works locally for me (but gets
noticeably slower as more threads are spawned). Given the structure of
the code there's no reason to test an arbitrary number of threads,
only 0, 1 and >=2 are special modes of operation.

Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
 t/t7810-grep.sh | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index daa906b9b0..561709ef6a 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -775,6 +775,22 @@ test_expect_success 'grep -W with userdiff' '
      test_cmp expected actual
 '

+for threads in $(test_seq 0 10)
+do
+     test_expect_success "grep --threads=$threads & -c grep.threads=$threads" "
+             git grep --threads=$threads . >actual.$threads &&
+             if test $threads -ge 1
+             then
+                     test_cmp actual.\$(($threads - 1)) actual.$threads
+             fi &&
+             git -c grep.threads=$threads grep . >actual.$threads &&
+             if test $threads -ge 1
+             then
+                     test_cmp actual.\$(($threads - 1)) actual.$threads
+             fi
+     "
+done
+
Is there a test condition to require PTHREADS?  Otherwise this might
break if git is compiled with NO_PTHREADS.
Good catch, this test works and I'll leave it like it is in a v2, but
explain it better in the commit message.

We just ignore --threads= with NO_PTHREADS, however later in this
series I introduce a warning for --threads when no threads are
supported, see "grep: given --threads with NO_PTHREADS=YesPlease,
warn".

But --threads=N still works, so this as a side-benefit tests that
--threads=N still works with NO_PTHREADS, and tests don't fail if we
spew to stderr.

The commit that adds the warning then tests for --threads getting a
warning with NO_THREADS=Y.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help