Re: [PATCH 3/3] test-lib: '--run' to run only specific tests

2014-05-06 Thread Junio C Hamano
Ilya Bobyr ilya.bo...@gmail.com writes:

 Allow better control of the set of tests that will be executed for a
 single test suite.  Mostly useful while debugging or developing as it
 allows to focus on a specific test.

 Signed-off-by: Ilya Bobyr ilya.bo...@gmail.com
 ---
 A number of minor changes according to the review comments.

I think the interaction between multiple selectors, especially when
some of them are negated, are much better explained in this version,
compared to the previous round in the README.

But I still think that the negation a feature that is unnecessary
and having it makes it harder to understand for users, especially
after reading this part:

 +If --run starts with an unprefixed number or range the initial
 +set of tests to run is empty. If the first item starts with '!'
 +all the tests are added to the initial set.  After initial set is
 +determined every test number or range is added or excluded from
 +the set one by one, from left to right.
 ...
 +As noted above, the test set is built going though items left to
 +right, so this:
 +
 +$ sh ./t9200-git-cvsexport-commit.sh --run='1-4 !3'
 +
 +will run tests 1, 2, and 4.  Items that comes later have higher
 +precendence.  It means that this:
 +
 +$ sh ./t9200-git-cvsexport-commit.sh --run='!3 1-4'
 +
 +would just run tests from 1 to 4, including 3.

The initial !3 means the same thing as 1-2,4-, and then 1-4 will
do what to that set?  The answer is It is added... wouldn't the
reader expect then that the result should be 1-, not 1-4?  I
myself wondered what would happen to the fifth test from your
description.  Has the text told the reader that t9200 test has only
four tests?

The need to explain better with longer description will reduce the
likelyhood that the feature is understood and correctly used.  When
you can write 1-2,4-, why accept 1-4 !3 and force yourself to
explain to people why that is different from !3 1-4?
--
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 3/3] test-lib: '--run' to run only specific tests

2014-05-06 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 The need to explain better with longer description will reduce the
 likelyhood that the feature is understood and correctly used.  When
 you can write 1-2,4-, why accept 1-4 !3 and force yourself to
 explain to people why that is different from !3 1-4?

By the way, having said all that, I would understand if the result
is made not depend on the order of selectors given to the option.

That is:

 - Find all the positive ones and form the positive set; if there is
   no positive ones, then make the positive set include everything.

 - Find all the negative ones and form a negative set.  The
   negative set can be an empty set if there is no negative
   selector.

 - Subtract the negative set from the positive set, and run only
   the tests in the resulting set.

Then 1-4 !3 and !3 1-4 would mean the same thing.  Explanation
of the semantics would be far simpler than we do it from left to
right.

One reason why the orders shouldn't matter is, unlike the print
dialog case, we won't run tests out of order when given 1 4 3 2.
We will still run 1-4.  So it would be natural for the readers to
expect that the orders they give selectors would not matter.
--
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 3/3] test-lib: '--run' to run only specific tests

2014-04-30 Thread Ilya Bobyr
On 4/23/2014 11:40 AM, Junio C Hamano wrote:
 Ilya Bobyr ilya.bo...@gmail.com writes:

 @@ -187,10 +192,70 @@ and either can match the t[0-9]{4} part to skip the 
 whole
  test, or t[0-9]{4} followed by .$number to say which
  particular test to skip.
  
 -Note that some tests in the existing test suite rely on previous
 -test item, so you cannot arbitrarily disable one and expect the
 -remainder of test to check what the test originally was intended
 -to check.
 +For an individual test suite --run could be used to specify that
 +only some tests should be run or that some tests should be
 +excluded from a run.
 +
 +The argument for --run is a list of individual test numbers or
 +ranges with an optional negation prefix that define what tests in
 +a test suite to include in the run.  A range is two numbers
 +separated with a dash and matches a range of tests with both ends
 +been included.  You may omit the first or the second number to
 +mean from the first test or up to the very last test
 +respectively.
 +
 +Optional prefix of '!' means that the test or a range of tests
 +should be excluded from the run.
 +
 +If --run starts with an unprefixed number or range the initial
 +set of tests to run is empty. If the first item starts with '!'
 +all the tests are added to the initial set.  After initial set is
 +determined every test number or range is added or excluded from
 +the set one by one, from left to right.
 +
 +Individual numbers or ranges could be separated either by a space
 +or a comma.
 +
 +For example, common case is to run several setup tests (1, 2, 3)
 +and then a specific test (21) that relies on that setup:
 +
 +$ sh ./t9200-git-cvsexport-commit.sh --run='1 2 3 21'
 +
 +or:
 +
 +$ sh ./t9200-git-cvsexport-commit.sh --run=1,2,3,21
 +
 +or:
 +
 +$ sh ./t9200-git-cvsexport-commit.sh --run='-3 21'
 Good and easily understandable examples. 

 +To run only tests up to a specific test (21), one could do this:
 +
 +$ sh ./t9200-git-cvsexport-commit.sh --run='1-21'
 +
 +or this:
 +
 +$ sh ./t9200-git-cvsexport-commit.sh --run='-21'
 These may be redundant, given that the reader would have to have
 grokked the earlier -3 21 already at this point.

The original idea was to show two most common use cases in the examples,
so that one could just copy/paste it.
I guess you are right that the second is a bit redundant now from the
standpoint of a person who is reading all of it.

I have reordered the examples.  Single range is simpler, it comes first
and then a more complicated example.

 +As noted above, the test set is built going though items left to
 +right, so this:
 +
 +$ sh ./t9200-git-cvsexport-commit.sh --run='1-4 !3'
 +
 +will run tests 1, 2, and 4.
 I do not quite understand what you mean by left to right; is that
 implementation detail necessary for the user of the feature, or is
 it talking about some limitation coming from the implementation?
 e.g. perhaps !3 1-4 would not work as people would expect do not
 run 3, but run tests from 1 thru 4 otherwise, and warning against
 having such an expectation that cannot be fulfilled?

I thought that it is something that you may want to understand if you
are going to build something complicated.  As I do not have a specific
use case, this is kind of a made up example.
The idea is that what is on the right overwrites what is on the left. 
I've added that sentence as an additional clarification, and your example.

 +You may use negation with ranges.  The following will run all
 +test as a test suite except from 7 upto 11:
 +
 +$ sh ./t9200-git-cvsexport-commit.sh --run='!7-11'
 Hmm, that is somewhat counter-intuitive or at least ambiguous.  I
 first thought you would be running everything but skipping 7 thru
 11, but your explanation is that it is equivalent to -6,8-11 (that
 is, to intersect set -11 and set !7).

Your expectation is correct.
A space or a comma is needed in order for !7 and -11 to be treated
separately.
I am not sure why did you read the description as -6,8-11.  There is a
typo in the sentence: s/as a/in the/.
I've changed that, but I would not object a better explanation of cause :)

 The above two illustrate the reason rather well why I said it would
 be better to avoid negation because it would complicate the mental
 model the user needs to form when using the feature.

I think that you do not have to use it if you do not need it.
It adds some expressiveness, is rather easy to implement and is already
there :)
I can remove it, of cause, but is it really necessary?

 +Some tests in a test suite rely on the previous tests performing
 +certain actions, specifically some tests are designated as
 +setup test, so you cannot _arbitrarily_ disable one test and
 +expect the rest to function correctly.
 What this text (moved from the top of this hunk) tells the reader
 applies to both the traditional t0123.4 and the new --run=1-3,5-
 syntaxes, but the new placement of it make it sound as if it is only
 for skipping 

Re: [PATCH 3/3] test-lib: '--run' to run only specific tests

2014-04-30 Thread Ilya Bobyr
On 4/23/2014 12:51 PM, Eric Sunshine wrote:
 On Tue, Apr 22, 2014 at 4:19 AM, Ilya Bobyr ilya.bo...@gmail.com wrote:
 Allow better control of the set of tests that will be executed for a
 single test suite.  Mostly useful while debugging or developing as it
 allows to focus on a specific test.

 Signed-off-by: Ilya Bobyr ilya.bo...@gmail.com
 ---
 diff --git a/t/README b/t/README
 index 6b93aca..2dac619 100644
 --- a/t/README
 +++ b/t/README
 +As noted above, the test set is built going though items left to
 +right, so this:
 +
 +$ sh ./t9200-git-cvsexport-commit.sh --run='1-4 !3'
 +
 +will run tests 1, 2, and 4.
 +
 +You may use negation with ranges.  The following will run all
 +test as a test suite except from 7 upto 11:
 s/upto/up to/
 ...or...
 s/upto/through/

Fixed.  Thanks.

 +$ sh ./t9200-git-cvsexport-commit.sh --run='!7-11'
 +
 +Some tests in a test suite rely on the previous tests performing
 +certain actions, specifically some tests are designated as
 +setup test, so you cannot _arbitrarily_ disable one test and
 +expect the rest to function correctly.
 +--run is mostly useful when you want to focus on a specific test
 +and know what you are doing.  Or when you want to run up to a
 +certain test.


  Naming Tests
 diff --git a/t/t-basic.sh b/t/t-basic.sh
 index ae8874e..e2589cc 100755
 --- a/t/t-basic.sh
 +++ b/t/t-basic.sh
 @@ -84,6 +97,18 @@ check_sub_test_lib_test () {
 )
  }

 +check_sub_test_lib_test_err () {
 +   name=$1 # stdin is the expected output output from the test
 +   # expecte error output is in descriptor 3
 s/expecte/expected/

Fixed.

 +   (
 +   cd $name 
 +   sed -e 's/^ //' -e 's/Z$//' expect.out 
 +   test_cmp expect.out out 
 +   sed -e 's/^ //' -e 's/Z$//' 3 expect.err 
 +   test_cmp expect.err err
 +   )
 +}
 +
  test_expect_success 'pretend we have a fully passing test suite' 
 run_sub_test_lib_test full-pass '3 passing tests' -\\EOF 
 for i in 1 2 3
 @@ -333,6 +358,329 @@ test_expect_success 'GIT_SKIP_TESTS sh pattern' 
 +test_expect_success '--run invalid range start' 
 +   run_sub_test_lib_test_err run-inv-range-start \
 +   '--run invalid range start' \
 +   --run='a-5' -\\EOF 
 +   test_expect_success \passing test #1\ 'true'
 +   test_done
 +   EOF
 +   check_sub_test_lib_test_err run-inv-range-start \
 +   -\\EOF_OUT 3-\\EOF_ERR
 +FATAL: Unexpected exit with code 1
 +   EOF_OUT
 +error: --run: range start should contain only digits: 'a-5'
 This reads rather strangely, as if it's attempting to give an example
 (after the colon) of a valid digit range, but then shows something
 that is not valid. Rewording it slightly can eliminate the ambiguity:

 error: --run: invalid non-numeric range start: 'a-5'

Changed.

 +   EOF_ERR
 +
 +
 +test_expect_success '--run invalid range end' 
 +   run_sub_test_lib_test_err run-inv-range-end \
 +   '--run invalid range end' \
 +   --run='1-z' -\\EOF 
 +   test_expect_success \passing test #1\ 'true'
 +   test_done
 +   EOF
 +   check_sub_test_lib_test_err run-inv-range-end \
 +   -\\EOF_OUT 3-\\EOF_ERR
 +FATAL: Unexpected exit with code 1
 +   EOF_OUT
 +error: --run: range end should contain only digits: '1-z'
 Ditto.

Fixed.

 +   EOF_ERR
 +
 +
 +test_expect_success '--run invalid selector' 
 +   run_sub_test_lib_test_err run-inv-selector \
 +   '--run invalid selector' \
 +   --run='1?' -\\EOF 
 +   test_expect_success \passing test #1\ 'true'
 +   test_done
 +   EOF
 +   check_sub_test_lib_test_err run-inv-selector \
 +   -\\EOF_OUT 3-\\EOF_ERR
 +FATAL: Unexpected exit with code 1
 +   EOF_OUT
 +error: --run: test selector should contain only digits: '1?'
 And here:

 error: --run: invalid non-digit in range selector: '1?'

 or something.

Changed to invalid non-digit in test selector.  This one is only shown
if it does not have a -, so it is probably not a range.

 +   EOF_ERR
 +
 +
 +
  test_set_prereq HAVEIT
  haveit=no
  test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '
 diff --git a/t/test-lib.sh b/t/test-lib.sh
 index e7d9c51..46ba513 100644
 --- a/t/test-lib.sh
 +++ b/t/test-lib.sh
 @@ -366,6 +374,100 @@ match_pattern_list () {
 return 1
  }

 +match_test_selector_list () {
 +   title=$1
 +   shift
 +   arg=$1
 +   shift
 +   test -z $1  return 0
 +
 +   # Both commas and spaces are accepted as separators
 +   OLDIFS=$IFS
 +   IFS='   ,'
 The comment mentions only space and comma, but the actual assigned IFS
 value also treats tabs as separators. Perhaps update the comment to
 say commas and whitespace.

I thought that tab is a space character =)  Changed it.

 +   set -- $1
 +   

Re: [PATCH 3/3] test-lib: '--run' to run only specific tests

2014-04-30 Thread Junio C Hamano
Ilya Bobyr ilya.bo...@gmail.com writes:

 The above two illustrate the reason rather well why I said it would
 be better to avoid negation because it would complicate the mental
 model the user needs to form when using the feature.

 I think that you do not have to use it if you do not need it.
 It adds some expressiveness, is rather easy to implement and is already
 there :)
 I can remove it, of cause, but is it really necessary?

An extra expressiveness that needs explanation and careful
thinking on the part of the user to pick the same world model you
picked among multiple valid world models is not necessarily a good
addition, so none of you do not have to use it, it's already
there, it is easy to implement is a valid argument.

If it weren't there, I wouldn't have had to wonder what the notation
meant, you wouldn't have had to explain it to me, and the most
importantly, nobody has to learn there is a subtle distinction
between !7 -11, !7-11 and !7- 11.
--
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 3/3] test-lib: '--run' to run only specific tests

2014-04-23 Thread Junio C Hamano
Ilya Bobyr ilya.bo...@gmail.com writes:

 @@ -187,10 +192,70 @@ and either can match the t[0-9]{4} part to skip the 
 whole
  test, or t[0-9]{4} followed by .$number to say which
  particular test to skip.
  
 -Note that some tests in the existing test suite rely on previous
 -test item, so you cannot arbitrarily disable one and expect the
 -remainder of test to check what the test originally was intended
 -to check.
 +For an individual test suite --run could be used to specify that
 +only some tests should be run or that some tests should be
 +excluded from a run.
 +
 +The argument for --run is a list of individual test numbers or
 +ranges with an optional negation prefix that define what tests in
 +a test suite to include in the run.  A range is two numbers
 +separated with a dash and matches a range of tests with both ends
 +been included.  You may omit the first or the second number to
 +mean from the first test or up to the very last test
 +respectively.
 +
 +Optional prefix of '!' means that the test or a range of tests
 +should be excluded from the run.
 +
 +If --run starts with an unprefixed number or range the initial
 +set of tests to run is empty. If the first item starts with '!'
 +all the tests are added to the initial set.  After initial set is
 +determined every test number or range is added or excluded from
 +the set one by one, from left to right.
 +
 +Individual numbers or ranges could be separated either by a space
 +or a comma.
 +
 +For example, common case is to run several setup tests (1, 2, 3)
 +and then a specific test (21) that relies on that setup:
 +
 +$ sh ./t9200-git-cvsexport-commit.sh --run='1 2 3 21'
 +
 +or:
 +
 +$ sh ./t9200-git-cvsexport-commit.sh --run=1,2,3,21
 +
 +or:
 +
 +$ sh ./t9200-git-cvsexport-commit.sh --run='-3 21'

Good and easily understandable examples. 

 +To run only tests up to a specific test (21), one could do this:
 +
 +$ sh ./t9200-git-cvsexport-commit.sh --run='1-21'
 +
 +or this:
 +
 +$ sh ./t9200-git-cvsexport-commit.sh --run='-21'

These may be redundant, given that the reader would have to have
grokked the earlier -3 21 already at this point.

 +As noted above, the test set is built going though items left to
 +right, so this:
 +
 +$ sh ./t9200-git-cvsexport-commit.sh --run='1-4 !3'
 +
 +will run tests 1, 2, and 4.

I do not quite understand what you mean by left to right; is that
implementation detail necessary for the user of the feature, or is
it talking about some limitation coming from the implementation?
e.g. perhaps !3 1-4 would not work as people would expect do not
run 3, but run tests from 1 thru 4 otherwise, and warning against
having such an expectation that cannot be fulfilled?

 +You may use negation with ranges.  The following will run all
 +test as a test suite except from 7 upto 11:
 +
 +$ sh ./t9200-git-cvsexport-commit.sh --run='!7-11'

Hmm, that is somewhat counter-intuitive or at least ambiguous.  I
first thought you would be running everything but skipping 7 thru
11, but your explanation is that it is equivalent to -6,8-11 (that
is, to intersect set -11 and set !7).

The above two illustrate the reason rather well why I said it would
be better to avoid negation because it would complicate the mental
model the user needs to form when using the feature.

 +Some tests in a test suite rely on the previous tests performing
 +certain actions, specifically some tests are designated as
 +setup test, so you cannot _arbitrarily_ disable one test and
 +expect the rest to function correctly.

What this text (moved from the top of this hunk) tells the reader
applies to both the traditional t0123.4 and the new --run=1-3,5-
syntaxes, but the new placement of it make it sound as if it is only
for skipping with --run, especially because the text before this
paragraph and also after this paragraph both apply only to --run.

 +--run is mostly useful when you want to focus on a specific test
 +and know what you are doing.  Or when you want to run up to a
 +certain test.

Likewise for and know what you are doing part.  I'd suggest
dropping that phrase from here, and/or make it part of the you
cannot randomly omit and expect later ones to work that covers both
ways to skip tests.

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 3/3] test-lib: '--run' to run only specific tests

2014-04-23 Thread Eric Sunshine
On Tue, Apr 22, 2014 at 4:19 AM, Ilya Bobyr ilya.bo...@gmail.com wrote:
 Allow better control of the set of tests that will be executed for a
 single test suite.  Mostly useful while debugging or developing as it
 allows to focus on a specific test.

 Signed-off-by: Ilya Bobyr ilya.bo...@gmail.com
 ---
 diff --git a/t/README b/t/README
 index 6b93aca..2dac619 100644
 --- a/t/README
 +++ b/t/README
 +As noted above, the test set is built going though items left to
 +right, so this:
 +
 +$ sh ./t9200-git-cvsexport-commit.sh --run='1-4 !3'
 +
 +will run tests 1, 2, and 4.
 +
 +You may use negation with ranges.  The following will run all
 +test as a test suite except from 7 upto 11:

s/upto/up to/
...or...
s/upto/through/

 +$ sh ./t9200-git-cvsexport-commit.sh --run='!7-11'
 +
 +Some tests in a test suite rely on the previous tests performing
 +certain actions, specifically some tests are designated as
 +setup test, so you cannot _arbitrarily_ disable one test and
 +expect the rest to function correctly.
 +--run is mostly useful when you want to focus on a specific test
 +and know what you are doing.  Or when you want to run up to a
 +certain test.


  Naming Tests
 diff --git a/t/t-basic.sh b/t/t-basic.sh
 index ae8874e..e2589cc 100755
 --- a/t/t-basic.sh
 +++ b/t/t-basic.sh
 @@ -84,6 +97,18 @@ check_sub_test_lib_test () {
 )
  }

 +check_sub_test_lib_test_err () {
 +   name=$1 # stdin is the expected output output from the test
 +   # expecte error output is in descriptor 3

s/expecte/expected/

 +   (
 +   cd $name 
 +   sed -e 's/^ //' -e 's/Z$//' expect.out 
 +   test_cmp expect.out out 
 +   sed -e 's/^ //' -e 's/Z$//' 3 expect.err 
 +   test_cmp expect.err err
 +   )
 +}
 +
  test_expect_success 'pretend we have a fully passing test suite' 
 run_sub_test_lib_test full-pass '3 passing tests' -\\EOF 
 for i in 1 2 3
 @@ -333,6 +358,329 @@ test_expect_success 'GIT_SKIP_TESTS sh pattern' 
 +test_expect_success '--run invalid range start' 
 +   run_sub_test_lib_test_err run-inv-range-start \
 +   '--run invalid range start' \
 +   --run='a-5' -\\EOF 
 +   test_expect_success \passing test #1\ 'true'
 +   test_done
 +   EOF
 +   check_sub_test_lib_test_err run-inv-range-start \
 +   -\\EOF_OUT 3-\\EOF_ERR
 +FATAL: Unexpected exit with code 1
 +   EOF_OUT
 +error: --run: range start should contain only digits: 'a-5'

This reads rather strangely, as if it's attempting to give an example
(after the colon) of a valid digit range, but then shows something
that is not valid. Rewording it slightly can eliminate the ambiguity:

error: --run: invalid non-numeric range start: 'a-5'

 +   EOF_ERR
 +
 +
 +test_expect_success '--run invalid range end' 
 +   run_sub_test_lib_test_err run-inv-range-end \
 +   '--run invalid range end' \
 +   --run='1-z' -\\EOF 
 +   test_expect_success \passing test #1\ 'true'
 +   test_done
 +   EOF
 +   check_sub_test_lib_test_err run-inv-range-end \
 +   -\\EOF_OUT 3-\\EOF_ERR
 +FATAL: Unexpected exit with code 1
 +   EOF_OUT
 +error: --run: range end should contain only digits: '1-z'

Ditto.

 +   EOF_ERR
 +
 +
 +test_expect_success '--run invalid selector' 
 +   run_sub_test_lib_test_err run-inv-selector \
 +   '--run invalid selector' \
 +   --run='1?' -\\EOF 
 +   test_expect_success \passing test #1\ 'true'
 +   test_done
 +   EOF
 +   check_sub_test_lib_test_err run-inv-selector \
 +   -\\EOF_OUT 3-\\EOF_ERR
 +FATAL: Unexpected exit with code 1
 +   EOF_OUT
 +error: --run: test selector should contain only digits: '1?'

And here:

error: --run: invalid non-digit in range selector: '1?'

or something.

 +   EOF_ERR
 +
 +
 +
  test_set_prereq HAVEIT
  haveit=no
  test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '
 diff --git a/t/test-lib.sh b/t/test-lib.sh
 index e7d9c51..46ba513 100644
 --- a/t/test-lib.sh
 +++ b/t/test-lib.sh
 @@ -366,6 +374,100 @@ match_pattern_list () {
 return 1
  }

 +match_test_selector_list () {
 +   title=$1
 +   shift
 +   arg=$1
 +   shift
 +   test -z $1  return 0
 +
 +   # Both commas and spaces are accepted as separators
 +   OLDIFS=$IFS
 +   IFS='   ,'

The comment mentions only space and comma, but the actual assigned IFS
value also treats tabs as separators. Perhaps update the comment to
say commas and whitespace.

 +   set -- $1
 +   IFS=$OLDIFS
 +
 +   # If the first selector is negative we include by default.
 +   include=
 +   case $1 in
 +   !*) include=t ;;
 +   esac
 +
 +   for selector
 +   do
 +   orig_selector=$selector
 +
 +

Unnecessary extra blank line.

 + 

Re: [PATCH 3/3] test-lib: '--run' to run only specific tests

2014-03-31 Thread Junio C Hamano
Eric Sunshine sunsh...@sunshineco.com writes:

 Since the relational operators are fairly self-explanatory, you could
 drop the prose explanation, though that might make it too cryptic:

 A number prefixed with '', '=', '', or '=' matches test
 numbers meeting the specified relation.

I would have to say that there is already an established pattern to
pick ranges that normal people understand well and it would be silly
to invent another more verbose way to express the same thing.  You
tell your Print Dialog which page to print with e.g. -4,7,9-12,15-,
not =4 7   

Would the same notation be insufficient for our purpose?  You do not
even have to worry about negation that way.
--
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 3/3] test-lib: '--run' to run only specific tests

2014-03-31 Thread David Tran
 Junio C Hamano gitster at pobox.com writes:

 
 I would have to say that there is already an established pattern to
 pick ranges that normal people understand well and it would be silly
 to invent another more verbose way to express the same thing.  You
 tell your Print Dialog which page to print with e.g. -4,7,9-12,15-,
 not =4 7   
 
 Would the same notation be insufficient for our purpose?  You do not
 even have to worry about negation that way.
 

That will do, especially if the numbers are in ascending order. We won't
have the odd cases of including a test and removing it later on. I think
we won't need a --neg flag to take the negation of the tests in that range?

--
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 3/3] test-lib: '--run' to run only specific tests

2014-03-30 Thread Eric Sunshine
On Fri, Mar 28, 2014 at 3:05 AM, Ilya Bobyr ilya.bo...@gmail.com wrote:
 On 3/27/2014 8:36 PM, Eric Sunshine wrote:
 On Thu, Mar 27, 2014 at 6:32 AM, Ilya Bobyr ilya.bo...@gmail.com wrote:
 Allow better control of the set of tests that will be executed for a
 single test suite.  Mostly useful while debugging or developing as it
 allows to focus on a specific test.

 Signed-off-by: Ilya Bobyr ilya.bo...@gmail.com
 ---
  No changes from the previous version.

  t/README |   65 ++-
  t/t-basic.sh |  233 
 ++
  t/test-lib.sh|   85 
  3 files changed, 379 insertions(+), 4 deletions(-)

 diff --git a/t/README b/t/README
 index 6b93aca..c911f89 100644
 --- a/t/README
 +++ b/t/README
 @@ -100,6 +100,10 @@ appropriately before running make.
 This causes additional long-running tests to be run (where
 available), for more exhaustive testing.

 +-r,--run=test numbers::
 Perhaps test-selection or something similar would be closer to the truth.

 I think your naming is better.  I will include it in the next version.

 +   This causes only specific tests to be included or excluded.  See
 This is phrased somewhat oddly, as if you had already been talking
 about tests being included or excluded, and that this option merely
 changes that selection. Perhaps something like:

 Run only the subset of tests indicated by test-selection.

 Will use that sentence as well :)

 +   section Skipping Tests below for test numbers syntax.
 +
  --valgrind=tool::
 Execute all Git binaries under valgrind tool tool and exit
 with status 126 on errors (just like regular tests, this will
 @@ -187,10 +191,63 @@ and either can match the t[0-9]{4} part to skip the 
 whole
  test, or t[0-9]{4} followed by .$number to say which
  particular test to skip.

 -Note that some tests in the existing test suite rely on previous
 -test item, so you cannot arbitrarily disable one and expect the
 -remainder of test to check what the test originally was intended
 -to check.
 +For an individual test suite --run could be used to specify that
 +only some tests should be run or that some tests should be
 +excluded from a run.
 +
 +--run argument is a list of patterns with optional prefixes that
 The argument for --run is a list...

 I think it could be either.  But I am not a native speaker.
 So, I will use your version :)

 +are matched against test numbers within the current test suite.
 +Supported pattern:
 +
 + - A number matches a test with that number.
 +
 + - sh metacharacters such as '*', '?' and '[]' match as usual in
 +   shell.
 +
 + - A number prefixed with '', '=', '', or '=' matches all
 +   tests 'before', 'before or including', 'after', or 'after or
 +   including' the specified one.
 I think you want and rather than or: before and including,
 after and including.

 I was thinking about an analogy to the corresponding mathematical
 operations here.  In mathematics, '=' is called less than or
 equal to[1].

 If you are thinking about test numbers you can say that you
 include a test if it has a number before or equal to the given
 one.  The sentence is A number prefixed with = matches all
 tests [with numbers] before or including the specified [number].

In English, it is idiomatic to say less than or equal for '=', and
greater than or equal for '='. before or including and after or
including are not used and sound odd. They also sound rather odd when
or is replaced with and. Using the idiomatic forms should be fine.

 Maybe if I change one to number it would be a bit less
 ambiguous.  Or even include all the omitted words.

Changing all tests to all test numbers and using the idiomatic forms gives:

A number prefixed with '', '=', '', or '=' matches,
respectively, all test numbers less than, less than or equal,
greater than, or greater than or equal to the specified one.

which isn't bad, though a bit verbose.

 I would not mind a completely different way to say it, but I am
 not yet sure that if I replace or with and it would make it
 a lot better.

Since the relational operators are fairly self-explanatory, you could
drop the prose explanation, though that might make it too cryptic:

A number prefixed with '', '=', '', or '=' matches test
numbers meeting the specified relation.

 [1] https://en.wikipedia.org/wiki/Inequality_%28mathematics%29

 +Optional prefixes are:
 +
 + - '+' or no prefix: test(s) matching the pattern are included in
 +   the run.
 +
 + - '-' or '!': test(s) matching the pattern are exluded from the
 +   run.
 I've been playing with --run, and I find that test selection is not
 especially intuitive. For instance, =16 !24 !20 is easier to
 reason about when written instead with ranges, such as 16-19 21-24,
 or perhaps 16-24 !20. Open-ended ranges make sense too: 5- means
 tests 5 through the last, and -5 means tests 1 through 5. (Yes, this
 conflicts with your use 

Re: [PATCH 3/3] test-lib: '--run' to run only specific tests

2014-03-28 Thread Ilya Bobyr
On 3/27/2014 8:36 PM, Eric Sunshine wrote:
 On Thu, Mar 27, 2014 at 6:32 AM, Ilya Bobyr ilya.bo...@gmail.com wrote:
 Allow better control of the set of tests that will be executed for a
 single test suite.  Mostly useful while debugging or developing as it
 allows to focus on a specific test.

 Signed-off-by: Ilya Bobyr ilya.bo...@gmail.com
 ---
  No changes from the previous version.

  t/README |   65 ++-
  t/t-basic.sh |  233 
 ++
  t/test-lib.sh|   85 
  3 files changed, 379 insertions(+), 4 deletions(-)

 diff --git a/t/README b/t/README
 index 6b93aca..c911f89 100644
 --- a/t/README
 +++ b/t/README
 @@ -100,6 +100,10 @@ appropriately before running make.
 This causes additional long-running tests to be run (where
 available), for more exhaustive testing.

 +-r,--run=test numbers::
 Perhaps test-selection or something similar would be closer to the truth.

I think your naming is better.  I will include it in the next version.

 +   This causes only specific tests to be included or excluded.  See
 This is phrased somewhat oddly, as if you had already been talking
 about tests being included or excluded, and that this option merely
 changes that selection. Perhaps something like:

 Run only the subset of tests indicated by test-selection.

Will use that sentence as well :)

 +   section Skipping Tests below for test numbers syntax.
 +
  --valgrind=tool::
 Execute all Git binaries under valgrind tool tool and exit
 with status 126 on errors (just like regular tests, this will
 @@ -187,10 +191,63 @@ and either can match the t[0-9]{4} part to skip the 
 whole
  test, or t[0-9]{4} followed by .$number to say which
  particular test to skip.

 -Note that some tests in the existing test suite rely on previous
 -test item, so you cannot arbitrarily disable one and expect the
 -remainder of test to check what the test originally was intended
 -to check.
 +For an individual test suite --run could be used to specify that
 +only some tests should be run or that some tests should be
 +excluded from a run.
 +
 +--run argument is a list of patterns with optional prefixes that
 The argument for --run is a list...

I think it could be either.  But I am not a native speaker.
So, I will use your version :)

 +are matched against test numbers within the current test suite.
 +Supported pattern:
 +
 + - A number matches a test with that number.
 +
 + - sh metacharacters such as '*', '?' and '[]' match as usual in
 +   shell.
 +
 + - A number prefixed with '', '=', '', or '=' matches all
 +   tests 'before', 'before or including', 'after', or 'after or
 +   including' the specified one.
 I think you want and rather than or: before and including,
 after and including.

I was thinking about an analogy to the corresponding mathematical
operations here.  In mathematics, '=' is called less than or
equal to[1].

If you are thinking about test numbers you can say that you
include a test if it has a number before or equal to the given
one.  The sentence is A number prefixed with = matches all
tests [with numbers] before or including the specified [number].

Maybe if I change one to number it would be a bit less
ambiguous.  Or even include all the omitted words.

I would not mind a completely different way to say it, but I am
not yet sure that if I replace or with and it would make it
a lot better.

[1] https://en.wikipedia.org/wiki/Inequality_%28mathematics%29

 +Optional prefixes are:
 +
 + - '+' or no prefix: test(s) matching the pattern are included in
 +   the run.
 +
 + - '-' or '!': test(s) matching the pattern are exluded from the
 +   run.
 I've been playing with --run, and I find that test selection is not
 especially intuitive. For instance, =16 !24 !20 is easier to
 reason about when written instead with ranges, such as 16-19 21-24,
 or perhaps 16-24 !20. Open-ended ranges make sense too: 5- means
 tests 5 through the last, and -5 means tests 1 through 5. (Yes, this
 conflicts with your use of '-' to mean negation, but you already have
 the perfectly serviceable '!' as an alias for negation.)

I completely agree that ranges allow one to express certain
obvious things much easier than just inequalities.  I was even
thinking on a possible syntax.  But then I realized that I do not
have a real use case for it.

The only use case that I had is described in the cover letter: to
run several setup tests and then the target test.  For that even
simple lists were enough and I was using that original version
with an environment variable.  After a conversation on the list I
thought that it would be nice to be able to say '', as it would
save typing several extra characters for cases like '1 2 3 4 25'.
While test suits that I've seen so far actually have no more than
two tests that do the setup.

The next use case that I could come up with was running up to a
specific test.  I was in a 

Re: [PATCH 3/3] test-lib: '--run' to run only specific tests

2014-03-27 Thread Eric Sunshine
On Thu, Mar 27, 2014 at 6:32 AM, Ilya Bobyr ilya.bo...@gmail.com wrote:
 Allow better control of the set of tests that will be executed for a
 single test suite.  Mostly useful while debugging or developing as it
 allows to focus on a specific test.

 Signed-off-by: Ilya Bobyr ilya.bo...@gmail.com
 ---
  No changes from the previous version.

  t/README |   65 ++-
  t/t-basic.sh |  233 
 ++
  t/test-lib.sh|   85 
  3 files changed, 379 insertions(+), 4 deletions(-)

 diff --git a/t/README b/t/README
 index 6b93aca..c911f89 100644
 --- a/t/README
 +++ b/t/README
 @@ -100,6 +100,10 @@ appropriately before running make.
 This causes additional long-running tests to be run (where
 available), for more exhaustive testing.

 +-r,--run=test numbers::

Perhaps test-selection or something similar would be closer to the truth.

 +   This causes only specific tests to be included or excluded.  See

This is phrased somewhat oddly, as if you had already been talking
about tests being included or excluded, and that this option merely
changes that selection. Perhaps something like:

Run only the subset of tests indicated by test-selection.

 +   section Skipping Tests below for test numbers syntax.
 +
  --valgrind=tool::
 Execute all Git binaries under valgrind tool tool and exit
 with status 126 on errors (just like regular tests, this will
 @@ -187,10 +191,63 @@ and either can match the t[0-9]{4} part to skip the 
 whole
  test, or t[0-9]{4} followed by .$number to say which
  particular test to skip.

 -Note that some tests in the existing test suite rely on previous
 -test item, so you cannot arbitrarily disable one and expect the
 -remainder of test to check what the test originally was intended
 -to check.
 +For an individual test suite --run could be used to specify that
 +only some tests should be run or that some tests should be
 +excluded from a run.
 +
 +--run argument is a list of patterns with optional prefixes that

The argument for --run is a list...

 +are matched against test numbers within the current test suite.
 +Supported pattern:
 +
 + - A number matches a test with that number.
 +
 + - sh metacharacters such as '*', '?' and '[]' match as usual in
 +   shell.
 +
 + - A number prefixed with '', '=', '', or '=' matches all
 +   tests 'before', 'before or including', 'after', or 'after or
 +   including' the specified one.

I think you want and rather than or: before and including,
after and including.

 +Optional prefixes are:
 +
 + - '+' or no prefix: test(s) matching the pattern are included in
 +   the run.
 +
 + - '-' or '!': test(s) matching the pattern are exluded from the
 +   run.

I've been playing with --run, and I find that test selection is not
especially intuitive. For instance, =16 !24 !20 is easier to
reason about when written instead with ranges, such as 16-19 21-24,
or perhaps 16-24 !20. Open-ended ranges make sense too: 5- means
tests 5 through the last, and -5 means tests 1 through 5. (Yes, this
conflicts with your use of '-' to mean negation, but you already have
the perfectly serviceable '!' as an alias for negation.)

 +If --run starts with '+' or unprefixed pattern the initial set of
 +tests to run is empty. If the first pattern starts with '-' or
 +'!' all the tests are added to the initial set.  After initial
 +set is determined every pattern, test number or range is added or
 +excluded from the set one by one, from left to right.
 +
 +For example, common case is to run several setup tests and then a
 +specific test that relies on that setup:

Perhaps be a bit more specific:

...run several setup tests (1, 2, 3) and then a
specific test (21) that relies...

 +$ sh ./t9200-git-cvsexport-commit.sh --run='1 2 3 21'
 +
 +or:
 +
 +$ sh ./t9200-git-cvsexport-commit.sh --run='4 21'

It might be clearer to say =3 rather than 4.

 +To run only tests up to a specific test one could do this:

s/specific test/specific test,/

Also perhaps:

...up to a specific test (21), one...

 +$ sh ./t9200-git-cvsexport-commit.sh --run='!=21'
 +
 +As noted above test set is build going though patterns left to

s/above/above,/
s/test set/the test set/
s/build/built/

As noted above, the test set is built...

 +right, so this:
 +
 +$ sh ./t9200-git-cvsexport-commit.sh --run='5 !3'
 +
 +will run tests 1, 2, and 4.
 +
 +Some tests in the existing test suite rely on previous test item,
 +so you cannot arbitrarily disable one and expect the remainder of
 +test to check what the test originally was intended to check.
 +--run is mostly useful when you want to focus on a specific test
 +and know what you are doing.  Or when you want to run up to a
 +certain test.


  Naming Tests
 diff --git a/t/test-lib.sh b/t/test-lib.sh
 index e035f36..63e481a 100644
 --- a/t/test-lib.sh
 +++ b/t/test-lib.sh
 @@ -191,6 +191,14 @@ do