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