Re: [PATCH 1/2] test-lib: tests skipped by GIT_SKIP_TESTS say so
On 3/3/2014 3:18 PM, Eric Sunshine wrote: On Mon, Mar 3, 2014 at 6:12 PM, Ilya Bobyr wrote: On 3/3/2014 2:59 PM, Eric Sunshine wrote: On Mon, Mar 3, 2014 at 5:24 AM, Ilya Bobyr wrote: We used to show "(missing )" next to tests skipped because they are specified in GIT_SKIP_TESTS. Use "(matched by GIT_SKIP_TESTS)" instead. Bikeshedding: That's pretty verbose. Perhaps just say "(excluded)"? The next patch adds another reason for the test to be skipped, so it seems reasonable to say why exactly. The patch actually makes it say "matched GIT_SKIP_TESTS". It looks OK on the console. Still just bikeshedding: That new message in patch #2 says "not in GIT_TEST_ONLY", but isn't "(excluded)" also applicable to that case? Is it important to be able to distinguish between the two "excluded" reasons? (No more bikeshedding for me.) Makes sense. I guess it is unlikely you would want to use both include and exclude filters in one run. On the other hand it seems nice to see the reason why it was skipped. Here are both options for comparison. "Longer": $ GIT_SKIP_TESTS='t.[236789] t0001.??' ./t0001-init.sh ok 1 - plain ok 2 # skip plain nested in bare (matched GIT_SKIP_TESTS) ok 3 # skip plain through aliased command, outside any git repo (matched GIT_SKIP_TESTS) not ok 4 - plain nested through aliased command # TODO known breakage not ok 5 - plain nested in bare through aliased command # TODO known breakage ok 6 # skip plain with GIT_WORK_TREE (matched GIT_SKIP_TESTS) and changed to "excluded": $ GIT_SKIP_TESTS='t.[236789] t0001.??' ./t0001-init.sh ok 1 - plain ok 2 # skip plain nested in bare (excluded) ok 3 # skip plain through aliased command, outside any git repo (excluded) not ok 4 - plain nested through aliased command # TODO known breakage not ok 5 - plain nested in bare through aliased command # TODO known breakage ok 6 # skip plain with GIT_WORK_TREE (excluded) P.S. It seems that now the whole interface may change :) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] test-lib: tests skipped by GIT_SKIP_TESTS say so
Eric Sunshine writes: > That new message in patch #2 says "not in GIT_TEST_ONLY", but isn't > "(excluded)" also applicable to that case? Is it important to be able > to distinguish between the two "excluded" reasons? An obvious solution is not to *have* two reasons in the first place ;-) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] test-lib: tests skipped by GIT_SKIP_TESTS say so
On Mon, Mar 3, 2014 at 6:12 PM, Ilya Bobyr wrote: > On 3/3/2014 2:59 PM, Eric Sunshine wrote: >> >> On Mon, Mar 3, 2014 at 5:24 AM, Ilya Bobyr wrote: >>> >>> We used to show "(missing )" next to tests skipped because they are >>> specified in GIT_SKIP_TESTS. Use "(matched by GIT_SKIP_TESTS)" instead. >> >> Bikeshedding: That's pretty verbose. Perhaps just say "(excluded)"? > > > The next patch adds another reason for the test to be skipped, so it seems > reasonable to say why exactly. > The patch actually makes it say "matched GIT_SKIP_TESTS". > It looks OK on the console. Still just bikeshedding: That new message in patch #2 says "not in GIT_TEST_ONLY", but isn't "(excluded)" also applicable to that case? Is it important to be able to distinguish between the two "excluded" reasons? (No more bikeshedding for me.) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] test-lib: tests skipped by GIT_SKIP_TESTS say so
Eric Sunshine writes: > On Mon, Mar 3, 2014 at 5:24 AM, Ilya Bobyr wrote: >> We used to show "(missing )" next to tests skipped because they are >> specified in GIT_SKIP_TESTS. Use "(matched by GIT_SKIP_TESTS)" instead. > > Bikeshedding: That's pretty verbose. Perhaps just say "(excluded)"? Sounds good, or at least better than matched GIT_SKIP_TESTS, to me ;-). Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] test-lib: tests skipped by GIT_SKIP_TESTS say so
On 3/3/2014 2:59 PM, Eric Sunshine wrote: On Mon, Mar 3, 2014 at 5:24 AM, Ilya Bobyr wrote: We used to show "(missing )" next to tests skipped because they are specified in GIT_SKIP_TESTS. Use "(matched by GIT_SKIP_TESTS)" instead. Bikeshedding: That's pretty verbose. Perhaps just say "(excluded)"? The next patch adds another reason for the test to be skipped, so it seems reasonable to say why exactly. The patch actually makes it say "matched GIT_SKIP_TESTS". It looks OK on the console. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] test-lib: tests skipped by GIT_SKIP_TESTS say so
On 3/3/2014 7:11 AM, Philip Oakley wrote:
From: "Ilya Bobyr"
We used to show "(missing )" next to tests skipped because they are
specified in GIT_SKIP_TESTS. Use "(matched by GIT_SKIP_TESTS)" instead.
The message below forgets the "by".
I'll fix the commit message. I think the output is long enough, while
"by" does not add any information.
Otherwise looks sensible.
Thanks for looking at it :)
---
t/test-lib.sh | 13 -
1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1531c24..89a405b 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -446,25 +446,28 @@ test_finish_ () {
test_skip () {
to_skip=
+ skipped_reason=
if match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS
then
to_skip=t
+ skipped_reason="matched GIT_SKIP_TESTS"
s/matched GIT_SKIP_TESTS/matched by GIT_SKIP_TESTS/
[...]
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] test-lib: tests skipped by GIT_SKIP_TESTS say so
On Mon, Mar 3, 2014 at 5:24 AM, Ilya Bobyr wrote:
> We used to show "(missing )" next to tests skipped because they are
> specified in GIT_SKIP_TESTS. Use "(matched by GIT_SKIP_TESTS)" instead.
Bikeshedding: That's pretty verbose. Perhaps just say "(excluded)"?
> ---
> t/test-lib.sh | 13 -
> 1 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 1531c24..89a405b 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -446,25 +446,28 @@ test_finish_ () {
>
> test_skip () {
> to_skip=
> + skipped_reason=
> if match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS
> then
> to_skip=t
> + skipped_reason="matched GIT_SKIP_TESTS"
> fi
> if test -z "$to_skip" && test -n "$test_prereq" &&
>! test_have_prereq "$test_prereq"
> then
> to_skip=t
> - fi
> - case "$to_skip" in
> - t)
> +
> of_prereq=
> if test "$missing_prereq" != "$test_prereq"
> then
> of_prereq=" of $test_prereq"
> fi
> -
> + skipped_reason="missing $missing_prereq${of_prereq}"
> + fi
> + case "$to_skip" in
> + t)
> say_color skip >&3 "skipping test: $@"
> - say_color skip "ok $test_count # skip $1 (missing
> $missing_prereq${of_prereq})"
> + say_color skip "ok $test_count # skip $1 ($skipped_reason)"
> : true
> ;;
> *)
> --
> 1.7.9
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] test-lib: tests skipped by GIT_SKIP_TESTS say so
From: "Ilya Bobyr"
We used to show "(missing )" next to tests skipped because they are
specified in GIT_SKIP_TESTS. Use "(matched by GIT_SKIP_TESTS)"
instead.
The message below forgets the "by". Otherwise looks sensible.
---
t/test-lib.sh | 13 -
1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1531c24..89a405b 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -446,25 +446,28 @@ test_finish_ () {
test_skip () {
to_skip=
+ skipped_reason=
if match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS
then
to_skip=t
+ skipped_reason="matched GIT_SKIP_TESTS"
s/matched GIT_SKIP_TESTS/matched by GIT_SKIP_TESTS/
fi
if test -z "$to_skip" && test -n "$test_prereq" &&
! test_have_prereq "$test_prereq"
then
to_skip=t
- fi
- case "$to_skip" in
- t)
+
of_prereq=
if test "$missing_prereq" != "$test_prereq"
then
of_prereq=" of $test_prereq"
fi
-
+ skipped_reason="missing $missing_prereq${of_prereq}"
+ fi
+ case "$to_skip" in
+ t)
say_color skip >&3 "skipping test: $@"
- say_color skip "ok $test_count # skip $1 (missing
$missing_prereq${of_prereq})"
+ say_color skip "ok $test_count # skip $1 ($skipped_reason)"
: true
;;
*)
--
1.7.9
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] test-lib: tests skipped by GIT_SKIP_TESTS say so
We used to show "(missing )" next to tests skipped because they are
specified in GIT_SKIP_TESTS. Use "(matched by GIT_SKIP_TESTS)" instead.
---
t/test-lib.sh | 13 -
1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1531c24..89a405b 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -446,25 +446,28 @@ test_finish_ () {
test_skip () {
to_skip=
+ skipped_reason=
if match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS
then
to_skip=t
+ skipped_reason="matched GIT_SKIP_TESTS"
fi
if test -z "$to_skip" && test -n "$test_prereq" &&
! test_have_prereq "$test_prereq"
then
to_skip=t
- fi
- case "$to_skip" in
- t)
+
of_prereq=
if test "$missing_prereq" != "$test_prereq"
then
of_prereq=" of $test_prereq"
fi
-
+ skipped_reason="missing $missing_prereq${of_prereq}"
+ fi
+ case "$to_skip" in
+ t)
say_color skip >&3 "skipping test: $@"
- say_color skip "ok $test_count # skip $1 (missing
$missing_prereq${of_prereq})"
+ say_color skip "ok $test_count # skip $1 ($skipped_reason)"
: true
;;
*)
--
1.7.9
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
