Re: [RFC/PATCH] Better control of the tests run by a test suite

2014-03-27 Thread Ilya Bobyr
On 3/24/2014 9:58 PM, Junio C Hamano wrote:
 Jeff King p...@peff.net writes:

 On Mon, Mar 24, 2014 at 01:49:44AM -0700, Ilya Bobyr wrote:

 Here are some examples of how functionality added by the patch
 could be used.  In order to run setup tests and then only a
 specific test (use case 1) one can do:

 $ ./t-init.sh --run='1 2 25'

 or:

 $ ./t-init.sh --run='3 25'

 ('=' is also supported, as well as '' and '=').
 I don't have anything against this in principle, but I suspect it will
 end up being a big pain to figure out which of the early tests are
 required to set up the state, and which are not. Having  makes
 specifying it easier, but you still have to read the test script to
 figure out which tests need to be run.
 Likewise.

The idea is that you will use that option when you know what
setup the test need.  And the case that I was targeting is when
you are the author of the test, because you are also writing the
relevant functionality or you are really familiar with the test
because you are, again, working on something in that area.

It does not mean you actually have to do it.  It is just a
possibility.

And as you mentioned,  helps in another case - when you do not
know enough about the test, but want to run it.  For example when
you are just starting with a failed test.

My experience, thought quite limited, is that it is very simple
to understand what the test needs and where it is prepared, if
you are actually adding new test to a test suite.  Or if you
spent some time figuring how specific test works.  I think this
is mostly because all the tests are rather simple.  Which is
definitely a good thing.

This is not for cases when you treat test suites as black boxes.
For example, when you are just checking someone else code.

 I wonder if it would make sense to auto-select tests that match a
 regex like set.?up|create? A while ago, Jonathan made a claim that
 this would cover most tests that are dependencies of other tests. I did
 not believe him, but looking into it, I recall that we did seem to have
 quite a few matching that pattern. If there were a good feature like
 this that gave us a reason to follow that pattern, I think people might
 fix the remainder
 This may be worth experimenting with, I would think.

I was also thinking about it a bit.  I do not have that much
knowledge on how all the tests are organized.  But I did see some
cases where this rule would fail.

One example would be t\t-basic.sh.  It could probably be
considered a very special test suite, but if you skip one of
these tests:

 - test runs if prerequisite is satisfied
 - unmet prerequisite causes test to be skipped

the test suite would just exit in the middle.  There is a number
of other tests that you do not want to skip for the same reason.
Also, in the same test suite showing tree with git ls-tree -r
is a setup test for the next one git ls-tree -r output for a
known tree.  And the same pattern is repeated for some other
tests.

I've also looked at t5601-clone.sh.  There is indeed a test
called setup at the very beginning.  But somewhere in the
middle there is a test called clone from .git file that creates
a folder used in two subsequent tests.

In t0001-init.sh, re-init on .git file creates a folder that
is used in the next test called re-init to update git link.

Maybe these are just some outliers, I do not know for sure.
These were the only test suites I've looked at so far.

I think that if there is a desire to support automatic setup for
tests maybe a rule could be introduced that a test must succeed,
if there is no breakage, if all the tests that match regex
'^(setup|cleanup)\' before it have been run.  It should not be
too complicated to create a target that would automate checking
of this rule.

I am not 100% sure that this kind of change is worth the trouble.
People who run individual tests should probably know why they are
doing it.  And as such that might know the prerequisites.

Otherwise I can not come up with a reason to run an individual
test.

On the other hand, the rule may add a bit more structure to the
tests and automated checking could enforce that.

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


[RFC/PATCH] Better control of the tests run by a test suite

2014-03-24 Thread Ilya Bobyr
Hello,

This is a second attempt on a functionality I proposed in

[PATCH 2/2] test-lib: GIT_TEST_ONLY to run only specific tests
http://www.mail-archive.com/git%40vger.kernel.org/msg44828.html

except that the implementation is quite different now.

I hope that I have accounted for the comments that were voiced so
far.  Let's see :)

The idea behind the change is that sometimes it is convenient to
run only certain tests from a test suite.  Specifically I am
thinking about the following two use cases:

 1. You are developing new functionality.  You add a test that
fails and then you add and modify code to make it pass.

 2. You have a failed test and you need to understand what is
wrong.

In the first case you when you run the test suite, you probably
want to run some setup tests and then only one test that you are
focused on.

For code written in C time between you make a change and see a
test result is considerably increased by the compilation.  But
for script code turn around time is mostly due to the run time of
the test suite itself. [1]

For the second case you actually want the test suite to stop
after the failing test, so that you can examine the trash
directory without any modifications from the subsequent tests.
You probably do not care about them.

In the previous patch I've added an environment variable to
control tests to be run in a test suite.  I thought that it would
be similar to an already existing GIT_SKIP_TESTS.  As I did not
provide a cover letter that caused some misunderstanding I think.

This patch adds new command line argument '--run' that accepts a
list of patterns and restrictions on the test numbers that would
be included or excluded from this run of the test suite.

During discussion of the previous patch there were some talks
about extending GIT_SKIP_TESTS syntax.  In particular here:

 That is
 
 GIT_SKIP_TESTS='t9??? !t91??'
 
 would skip nine-thousand series, but would run 91xx series, and all
 the others are not excluded.
 
 Simple rules to consider:
 
  - If the list consists of _only_ negated patterns, pretend that
there is unless otherwise specified with negatives, skip all
tests, i.e. treat GIT_SKIP_TESTS='!t91??' just the same way you
would treat GIT_SKIP_TESTS='* !t91??'.
 
  - The orders should not matter for simplicity of the semantics;
before running each test, check if it matches any negative (and
run it if it matches, without looking at any positives), and
otherwise check if it matches any positive (and skip it if it
does not).
 
 Hmm?

http://www.mail-archive.com/git%40vger.kernel.org/msg44922.html


I've used that as a basis, but the end result is somewhat
different.  Plus I've added boundary checks as in 123.

Here are some examples of how functionality added by the patch
could be used.  In order to run setup tests and then only a
specific test (use case 1) one can do:

$ ./t-init.sh --run='1 2 25'

or:

$ ./t-init.sh --run='3 25'

('=' is also supported, as well as '' and '=').

In order to run up to a specific test (use case 2) one can do:

$ ./t-init.sh --run='=34'

or:

$ ./t-init.sh --run='!34'

Simple semantics described above is easy to implement, but at the
same time is probably not that convenient.  Rules implemented by
the patch are as follows:

 - Order does matter.  Whatever is specified on the right has
   higher precedence.

 - When the first pattern is positive the initial set of the
   tests to be run is empty.  You are adding to an empty set as
   in '1 2 25'.

   When the first pattern is negative the initial set of the
   tests to run contains all the tests.  You are subtracting
   from that set as in '!34'.

It seems that for simple cases that gives simple syntax and is
almost unbiased if you think about preferring inclusion over
exclusion.  While it is unlikely it also allows for complicated
expressions.  And the implementation is quite simple as well.

Main functionality is in the third patch.  First two are just
minor fixes in related parts of the code.

P.S. I did not reply to the previous patch thread as this one is
quite different.


[1] Here are some times I see on Cygin:

$ touch builtin/rev-parse.c

$ time make
...

real0m10.382s
user0m3.829s
sys 0m5.269s

Running the t-init.sh test suite is like this:

$ time ./t0001-init.sh
[...]
1..36

real0m6.693s
user0m1.505s
sys 0m3.937s

If I run only the 1, 2, 4 and 5th tests, it only half the time to
run the tests:

$ time GIT_SKIP_TESTS='t0001.[36789] t0001.??' ./t0001-init.sh
[...]
1..36

real0m3.313s
user0m0.769s
sys 0m1.844s 

Overall the change is from 17 to 14 seconds it is not that big.

If you only consider the test suite, as you do while you develop
an sh based tool, for example, the change is from 6.6 to 3.3
seconds.  That is quite noticeable.


 t/README | 

Re: [RFC/PATCH] Better control of the tests run by a test suite

2014-03-24 Thread Jeff King
On Mon, Mar 24, 2014 at 01:49:44AM -0700, Ilya Bobyr wrote:

 Here are some examples of how functionality added by the patch
 could be used.  In order to run setup tests and then only a
 specific test (use case 1) one can do:
 
 $ ./t-init.sh --run='1 2 25'
 
 or:
 
 $ ./t-init.sh --run='3 25'
 
 ('=' is also supported, as well as '' and '=').

I don't have anything against this in principle, but I suspect it will
end up being a big pain to figure out which of the early tests are
required to set up the state, and which are not. Having  makes
specifying it easier, but you still have to read the test script to
figure out which tests need to be run.

I wonder if it would make sense to auto-select tests that match a
regex like set.?up|create? A while ago, Jonathan made a claim that
this would cover most tests that are dependencies of other tests. I did
not believe him, but looking into it, I recall that we did seem to have
quite a few matching that pattern. If there were a good feature like
this that gave us a reason to follow that pattern, I think people might
fix the remainder

-Peff
--
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: [RFC/PATCH] Better control of the tests run by a test suite

2014-03-24 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Mon, Mar 24, 2014 at 01:49:44AM -0700, Ilya Bobyr wrote:

 Here are some examples of how functionality added by the patch
 could be used.  In order to run setup tests and then only a
 specific test (use case 1) one can do:
 
 $ ./t-init.sh --run='1 2 25'
 
 or:
 
 $ ./t-init.sh --run='3 25'
 
 ('=' is also supported, as well as '' and '=').

 I don't have anything against this in principle, but I suspect it will
 end up being a big pain to figure out which of the early tests are
 required to set up the state, and which are not. Having  makes
 specifying it easier, but you still have to read the test script to
 figure out which tests need to be run.

Likewise.

 I wonder if it would make sense to auto-select tests that match a
 regex like set.?up|create? A while ago, Jonathan made a claim that
 this would cover most tests that are dependencies of other tests. I did
 not believe him, but looking into it, I recall that we did seem to have
 quite a few matching that pattern. If there were a good feature like
 this that gave us a reason to follow that pattern, I think people might
 fix the remainder

This may be worth experimenting with, I would think.
--
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