| Message ID | 41e2528e83ba7087c9d21f0b15efed416f1512f8.1602616786.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Make test selection easier by specifying description substrings instead of just numeric counters | expand |
On Tue, Oct 13, 2020 at 07:19:44PM +0000, Elijah Newren via GitGitGadget wrote: > Many of our test scripts have several "setup" tests. It's a lot easier > to say > > ./t0050-filesystem.sh --run=setup,9 I like this direction very well. There was a small discussion recently that we might be better off dropping test script numbers entirely, and allowing selection of script names by word-hierarchy or regex, too. That's mostly orthogonal to what you're doing here, but I think this is taking us in a good direction there. > @@ -819,9 +821,8 @@ match_test_selector_list () { > *) > if expr "z$selector" : "z[0-9]*[^0-9]" >/dev/null > then > - echo "error: $title: invalid non-numeric in test" \ > - "selector: '$orig_selector'" >&2 > - exit 1 > + echo "$title" | grep -q "$selector" && return > + continue > fi I like that you allow regexes. It's unfortunate that the skip-check costs us a process in every test. It may not be that big a deal since we only pay it if you use a non-numeric selector. But I wonder if there's any reason not to use "expr" here, as well. -Peff
> > @@ -819,9 +821,8 @@ match_test_selector_list () { > > *) > > if expr "z$selector" : "z[0-9]*[^0-9]" >/dev/null > > then > > - echo "error: $title: invalid non-numeric in test" \ > > - "selector: '$orig_selector'" >&2 > > - exit 1 > > + echo "$title" | grep -q "$selector" && return > > + continue > > fi > > I like that you allow regexes. It's unfortunate that the skip-check > costs us a process in every test. It may not be that big a deal since we > only pay it if you use a non-numeric selector. But I wonder if there's > any reason not to use "expr" here, as well. If you define the pattern is not regexp but is glob, you can use case/esac to do this without any forking. Your expr may well be built-in, though.
On Wed, Oct 14, 2020 at 10:04 AM Jeff King <peff@peff.net> wrote: > > On Tue, Oct 13, 2020 at 07:19:44PM +0000, Elijah Newren via GitGitGadget wrote: > > > Many of our test scripts have several "setup" tests. It's a lot easier > > to say > > > > ./t0050-filesystem.sh --run=setup,9 > > I like this direction very well. > > There was a small discussion recently that we might be better off > dropping test script numbers entirely, and allowing selection of script > names by word-hierarchy or regex, too. That's mostly orthogonal to what > you're doing here, but I think this is taking us in a good direction > there. > > > @@ -819,9 +821,8 @@ match_test_selector_list () { > > *) > > if expr "z$selector" : "z[0-9]*[^0-9]" >/dev/null > > then > > - echo "error: $title: invalid non-numeric in test" \ > > - "selector: '$orig_selector'" >&2 > > - exit 1 > > + echo "$title" | grep -q "$selector" && return > > + continue > > fi > > I like that you allow regexes. It's unfortunate that the skip-check > costs us a process in every test. It may not be that big a deal since we > only pay it if you use a non-numeric selector. But I wonder if there's > any reason not to use "expr" here, as well. I originally used [[ $title =~ "$selector" ]] in order to avoid the sub-shell...but that was bash-specific. I briefly looked to see if there was a shell portable way to handle regexes, wasn't having much luck, and decided that this is only likely to arise when people are passing --run and thus only running a single script and they avoid all the subprocesses that would have been invoked inside the test, so it's still a big net win overall. Does expr handle regexes, portably? Or are you suggesting dropping the regex handling and limit it to substring matching? In either case, does using expr save us anything (isn't expr a shell command)?
> are you suggesting dropping the regex handling and limit it to > substring matching? In either case, does using expr save us anything > (isn't expr a shell command)? I had something along this line in mind, not to do a regex but a glob. case "$title" in $selector) found=1 ;; esac
On Wed, Oct 14, 2020 at 10:46:15AM -0700, Junio C Hamano wrote: > > I like that you allow regexes. It's unfortunate that the skip-check > > costs us a process in every test. It may not be that big a deal since we > > only pay it if you use a non-numeric selector. But I wonder if there's > > any reason not to use "expr" here, as well. > > If you define the pattern is not regexp but is glob, you can use > case/esac to do this without any forking. Yes, that would probably be OK for most purposes, though I admit my real love for regex support is the ability to use "." instead of space to avoid quoting arguments. ;) Globs may make some real patterns slightly simpler, though. I imagine that the "setup" example may need to be "set.?up" or "set.*up" in practice. Which is only "set*up" as a glob (I also don't have a problem standardizing on one spelling as people find cases). > Your expr may well be built-in, though. Yeah, that was my assumption, though I didn't bother to test it. Having done so, it looks like it's not a built-in either in dash or bash. So switching to it from grep may be buying less in practice than I thought. We're also running a ton of exprs earlier in the function. Running: strace -f -e execve -o foo.out ./t0003-attributes.sh --run=10 appears to exec expr 65 times. There are only 103 execves total in the whole run, so that's more than half of them! It might be worth seeing if some of those could do globbing via case/esac. Repeating without "--run" yields 39 exprs out of 492 execs. So that's less abysmal. Most of those are from test_oid_cache. -Peff
On Wed, Oct 14, 2020 at 10:50:03AM -0700, Elijah Newren wrote: > > > @@ -819,9 +821,8 @@ match_test_selector_list () { > > > *) > > > if expr "z$selector" : "z[0-9]*[^0-9]" >/dev/null > > > then > > > - echo "error: $title: invalid non-numeric in test" \ > > > - "selector: '$orig_selector'" >&2 > > > - exit 1 > > > + echo "$title" | grep -q "$selector" && return > > > + continue > > > fi > > > > I like that you allow regexes. It's unfortunate that the skip-check > > costs us a process in every test. It may not be that big a deal since we > > only pay it if you use a non-numeric selector. But I wonder if there's > > any reason not to use "expr" here, as well. > > I originally used [[ $title =~ "$selector" ]] in order to avoid the > sub-shell...but that was bash-specific. I briefly looked to see if > there was a shell portable way to handle regexes, wasn't having much > luck, and decided that this is only likely to arise when people are > passing --run and thus only running a single script and they avoid all > the subprocesses that would have been invoked inside the test, so it's > still a big net win overall. Does expr handle regexes, portably? Or > are you suggesting dropping the regex handling and limit it to > substring matching? In either case, does using expr save us anything > (isn't expr a shell command)? There's an expr command doing a regex match in the diff context quoted above. :) I was wrong that it would save us a process, though. I thought it was a builtin in modern shells, but it doesn't appear to be in either dash or bash. So there's little reason to prefer it over grep here (switching to globs could be done with case and would save a process). -Peff
On Wed, Oct 14, 2020 at 11:09 AM Jeff King <peff@peff.net> wrote: > > On Wed, Oct 14, 2020 at 10:50:03AM -0700, Elijah Newren wrote: > > > > > @@ -819,9 +821,8 @@ match_test_selector_list () { > > > > *) > > > > if expr "z$selector" : "z[0-9]*[^0-9]" >/dev/null > > > > then > > > > - echo "error: $title: invalid non-numeric in test" \ > > > > - "selector: '$orig_selector'" >&2 > > > > - exit 1 > > > > + echo "$title" | grep -q "$selector" && return > > > > + continue > > > > fi > > > > > > I like that you allow regexes. It's unfortunate that the skip-check > > > costs us a process in every test. It may not be that big a deal since we > > > only pay it if you use a non-numeric selector. But I wonder if there's > > > any reason not to use "expr" here, as well. > > > > I originally used [[ $title =~ "$selector" ]] in order to avoid the > > sub-shell...but that was bash-specific. I briefly looked to see if > > there was a shell portable way to handle regexes, wasn't having much > > luck, and decided that this is only likely to arise when people are > > passing --run and thus only running a single script and they avoid all > > the subprocesses that would have been invoked inside the test, so it's > > still a big net win overall. Does expr handle regexes, portably? Or > > are you suggesting dropping the regex handling and limit it to > > substring matching? In either case, does using expr save us anything > > (isn't expr a shell command)? > > There's an expr command doing a regex match in the diff context quoted > above. :) Oh, indeed. How'd I miss that? > I was wrong that it would save us a process, though. I thought it was a > builtin in modern shells, but it doesn't appear to be in either dash or > bash. So there's little reason to prefer it over grep here (switching to > globs could be done with case and would save a process). I guess it's at least slightly more consistent with the surrounding code. Oh, and I found a bug in that my code was not handling negated substrings correctly, e.g. ./test-script.sh --run="!rename" would run all the tests instead of excluding the ones that had "rename" as a substring. So I'll send a reroll fixing that.
On Wed, Oct 14, 2020 at 10:57 AM Junio C Hamano <gitster@pobox.com> wrote: > > > are you suggesting dropping the regex handling and limit it to > > substring matching? In either case, does using expr save us anything > > (isn't expr a shell command)? > > I had something along this line in mind, not to do a regex but a glob. > > case "$title" in $selector) found=1 ;; esac Interesting. Since it needs to handle substring searching (e.g. ./test-script.sh --run=setup,rename), I think this would need to be tweaked to be case "$title" in *${selector}*) include=$positive ;; esac That'd probably be good enough for most cases, but I'm still inclined to just pay the subprocess cost in order to allow regexes. If someone is specifying --run, they are going to be skipping many of the tests (and thus a whole code block with dozens or even hundreds of execs per test skipped), more than making up for the one extra exec per test. And if someone doesn't specify --run, there is no extra cost.
> > If you define the pattern is not regexp but is glob, you can use > > case/esac to do this without any forking. > > Yes, that would probably be OK for most purposes, though I admit my real > love for regex support is the ability to use "." instead of space to > avoid quoting arguments. ;) I use "?" for the same purpose for globs. For things that are casual, I find that it tends to make the end-user (read: my) experience simpler to use globs than to use regexp, largely for your ".*" vs "*" reasons.
On Wed, Oct 14, 2020 at 12:24 PM Junio C Hamano <gitster@pobox.com> wrote: > > > > If you define the pattern is not regexp but is glob, you can use > > > case/esac to do this without any forking. > > > > Yes, that would probably be OK for most purposes, though I admit my real > > love for regex support is the ability to use "." instead of space to > > avoid quoting arguments. ;) > > I use "?" for the same purpose for globs. For things that are casual, > I find that > it tends to make the end-user (read: my) experience simpler to use globs than > to use regexp, largely for your ".*" vs "*" reasons. Oh, I thought you were arguing for globs over regexes here just due to performance reasons. Most of my uses of this feature so far are just substring matches, so either globs or regexes would be fine for what I've done so far. Mostly, I was leaning towards regexes because I figured they were a bit more flexible and perhaps there might arise a usecase where that would be handy (plus Peff seemed to like them). But if you find the normal usecase easier with globs, maybe I should let you and Peff argue it out and then I'll resubmit with the preferred solution?
On Wed, Oct 14, 2020 at 12:41:00PM -0700, Elijah Newren wrote: > > I use "?" for the same purpose for globs. For things that are casual, > > I find that > > it tends to make the end-user (read: my) experience simpler to use globs than > > to use regexp, largely for your ".*" vs "*" reasons. > > Oh, I thought you were arguing for globs over regexes here just due to > performance reasons. Most of my uses of this feature so far are just > substring matches, so either globs or regexes would be fine for what > I've done so far. Mostly, I was leaning towards regexes because I > figured they were a bit more flexible and perhaps there might arise a > usecase where that would be handy (plus Peff seemed to like them). > But if you find the normal usecase easier with globs, maybe I should > let you and Peff argue it out and then I'll resubmit with the > preferred solution? Thinking on it more, globs are probably a more natural fit here. Especially if we do eventually allow selection of scripts by name, too (e.g., for GIT_SKIP_TESTS), matching those filenames with a glob would match what happens on the command-line. -Peff
Elijah Newren <newren@gmail.com> writes: > On Wed, Oct 14, 2020 at 12:24 PM Junio C Hamano <gitster@pobox.com> wrote: >> >> > > If you define the pattern is not regexp but is glob, you can use >> > > case/esac to do this without any forking. >> > >> > Yes, that would probably be OK for most purposes, though I admit my real >> > love for regex support is the ability to use "." instead of space to >> > avoid quoting arguments. ;) >> >> I use "?" for the same purpose for globs. For things that are casual, >> I find that >> it tends to make the end-user (read: my) experience simpler to use globs than >> to use regexp, largely for your ".*" vs "*" reasons. > > Oh, I thought you were arguing for globs over regexes here just due to > performance reasons. Heh, but no, not me. You and Peff were the ones who were talking about performance by counting forks. I more often than others come from usability's point of view. When we are not dealing with uncontrollable and unbounded possiblity of end-user generated contents, I find it easier to forego the power and flexibility of regexp and instead settle on simpler globs---matching against the test titles is a good example use case, I would imagine. But if we are exposing regexp to those who run and debug test scripts already, I am perfectly fine with using regexp with 'expr'. Thanks.
Jeff King <peff@peff.net> writes: > Thinking on it more, globs are probably a more natural fit here. > Especially if we do eventually allow selection of scripts by name, too > (e.g., for GIT_SKIP_TESTS), matching those filenames with a glob would > match what happens on the command-line. Ah, I didn't think of "match with filenames" but yes that is a good complement to our "match with test titles" to be consistent with. Thanks.
Elijah Newren <newren@gmail.com> writes: > On Wed, Oct 14, 2020 at 10:57 AM Junio C Hamano <gitster@pobox.com> wrote: >> >> > are you suggesting dropping the regex handling and limit it to >> > substring matching? In either case, does using expr save us anything >> > (isn't expr a shell command)? >> >> I had something along this line in mind, not to do a regex but a glob. >> >> case "$title" in $selector) found=1 ;; esac > > Interesting. Since it needs to handle substring searching (e.g. > ./test-script.sh --run=setup,rename), I think this would need to be > tweaked to be > case "$title" in *${selector}*) include=$positive ;; esac Yup, of course we need to un-anchor with surrounding asterisks. Thanks.
Jeff King <peff@peff.net> writes: > On Tue, Oct 13, 2020 at 07:19:44PM +0000, Elijah Newren via GitGitGadget wrote: > >> Many of our test scripts have several "setup" tests. It's a lot easier >> to say >> >> ./t0050-filesystem.sh --run=setup,9 > > I like this direction very well. > > There was a small discussion recently that we might be better off > dropping test script numbers entirely,... I think I missed that one. A pointer, if you have one handy? Thanks.
On Thu, Oct 15, 2020 at 09:20:13AM -0700, Junio C Hamano wrote: > Jeff King <peff@peff.net> writes: > > > On Tue, Oct 13, 2020 at 07:19:44PM +0000, Elijah Newren via GitGitGadget wrote: > > > >> Many of our test scripts have several "setup" tests. It's a lot easier > >> to say > >> > >> ./t0050-filesystem.sh --run=setup,9 > > > > I like this direction very well. > > > > There was a small discussion recently that we might be better off > > dropping test script numbers entirely,... > > I think I missed that one. A pointer, if you have one handy? The sub-thread between me and Jonathan starting here: https://lore.kernel.org/git/20201005082448.GB2862927@coredump.intra.peff.net/ but specifically: https://lore.kernel.org/git/20201005084946.GE2862927@coredump.intra.peff.net/ -Peff
Jeff King <peff@peff.net> writes: > On Thu, Oct 15, 2020 at 09:20:13AM -0700, Junio C Hamano wrote: > >> Jeff King <peff@peff.net> writes: >> >> > On Tue, Oct 13, 2020 at 07:19:44PM +0000, Elijah Newren via GitGitGadget wrote: >> > >> >> Many of our test scripts have several "setup" tests. It's a lot easier >> >> to say >> >> >> >> ./t0050-filesystem.sh --run=setup,9 >> > >> > I like this direction very well. >> > >> > There was a small discussion recently that we might be better off >> > dropping test script numbers entirely,... >> >> I think I missed that one. A pointer, if you have one handy? > > The sub-thread between me and Jonathan starting here: > > https://lore.kernel.org/git/20201005082448.GB2862927@coredump.intra.peff.net/ > > but specifically: > > https://lore.kernel.org/git/20201005084946.GE2862927@coredump.intra.peff.net/ Ah, I see. I actually do use "git ls-files t/ | grep ..." to look for tests that are relevant to the issue I have at hand quite often, so unlike what Jonathan said in the thread, having a good name does matter to me. As far as I can tell, the numbers in the test names serve only two purposes. One is as a unique key to avoid colliding in the test result aggrevation database, and the other is as a unique key to use in GIT_SKIP_TESTS (which in turn is used by the Meta/Make wrapper I use, found on the todo branch, like 'Meta/Make --test=0050,1400 test'). I would be heavily inconvenienced if we decide to remove numbers becuase it would rob the latter use case from me, but I'd survive if we just are going to lift the requirement that numbers must be unique. I may end up running irrelevant 0050 and 1400 when the tests I really want to run are the other 0050 and 1400 with "--test=0050,1400", but when I am trying to run only 2 among 900+ scripts, running 2 extra ones I didn't have to run only because their prefix collide is still much better and tolerable. Thanks.
On Thu, Oct 15, 2020 at 01:08:22PM -0700, Junio C Hamano wrote: > > The sub-thread between me and Jonathan starting here: > > > > https://lore.kernel.org/git/20201005082448.GB2862927@coredump.intra.peff.net/ > > > > but specifically: > > > > https://lore.kernel.org/git/20201005084946.GE2862927@coredump.intra.peff.net/ > > Ah, I see. I actually do use "git ls-files t/ | grep ..." to look > for tests that are relevant to the issue I have at hand quite often, > so unlike what Jonathan said in the thread, having a good name does > matter to me. I'm not sure where he suggests worse names. I'd think if anything we'd have better names, because they'd be even more meaningful (if people start using them for test selectors). FWIW, I also grep like that when looking for scripts. > As far as I can tell, the numbers in the test names serve only two > purposes. > > One is as a unique key to avoid colliding in the test result > aggrevation database, and the other is as a unique key to use in > GIT_SKIP_TESTS (which in turn is used by the Meta/Make wrapper I > use, found on the todo branch, like 'Meta/Make --test=0050,1400 > test'). Yeah, that sounds right. We also frequently talk about "t5601" in human-readable messages. I often use that to either tab-complete or run "./t5601-*". So they do serve as short unique identifiers there. You could tell me "t-foo-bar-baz" and that would similarly be unique, but it's not as short. :) They do serve as unique keys in places like test-results/, but really the whole test name is the key. Most of them have unique text descriptions. If I do: git ls-files t/ | perl -lne '/t[0-9]{4}-(.*).sh$/ and print "$1"' | sort | uniq -d that only yields two duplicates (and we'd probably be better off giving them unique names anyway). > I would be heavily inconvenienced if we decide to remove numbers > becuase it would rob the latter use case from me, but I'd survive if > we just are going to lift the requirement that numbers must be > unique. My plan is that you'd be able to say: Meta/make --test=filesystem,update-ref for t0050 and t1400 respectively. Or more interesting things like "git-svn-*" to skip all of the t91xx tests (or "git-svn/*" if we split them by directories). > I may end up running irrelevant 0050 and 1400 when the > tests I really want to run are the other 0050 and 1400 with > "--test=0050,1400", but when I am trying to run only 2 among 900+ > scripts, running 2 extra ones I didn't have to run only because > their prefix collide is still much better and tolerable. I think you could already do this in general. The only thing standing in the way is test-lint-duplicates. I added that because I thought it would be better to catch the duplicates early rather than get confused by them later when you can't uniquely identify a test. But in practice I do feel like I spend more time de-duping tests when I rebase or merge (because my branch raced with somebody else allocating the test number) than I ever did dealing with unexpectedly skipped tests. I definitely don't want to make anybody's life harder. But if the numbers aren't unique and the text connected to them is, then it seems like we should just use the text primarily. -Peff
Jeff King <peff@peff.net> writes: >> Ah, I see. I actually do use "git ls-files t/ | grep ..." to look >> for tests that are relevant to the issue I have at hand quite often, >> so unlike what Jonathan said in the thread, having a good name does >> matter to me. > > I'm not sure where he suggests worse names. I'd think if anything we'd > have better names, because they'd be even more meaningful (if people > start using them for test selectors). FWIW, I also grep like that when > looking for scripts. I didn't mean Jonathan suggested worse names. Unlike "I don't tend to discover test scripts based on their filename", which was what Jonathan said, I do look for tests based on their filename, so having a good name matters (on the other hand, if you are the kind of person who does not look for them by name, the naming may not matter to you). > My plan is that you'd be able to say: > > Meta/make --test=filesystem,update-ref > > for t0050 and t1400 respectively. Or more interesting things like > "git-svn-*" to skip all of the t91xx tests (or "git-svn/*" if we split > them by directories). Most of the time, I am focused on running only the selected few, not excluding a few known-broken ones. For example, with an round of integration, I know only two tests fail, and after I tried to fix the bug in the series, I want to retry these two tests and nothing else to see if the attempted fix patches the breakage up (of course I need to run the remainder of the tests to ensure there is no regression, but that is just the matter of running "make test" to run the whole thing again). As long as it is known that "filesystem" and "update-ref" can serve as tokens to uniquely identify these two tests, it would be fine for my purpose. But 0050 (under the rule that numbering must be unique) would give me such an assurance much better without having to look at any other test file. The word "filesystem"? Unless we have a rule that we can use each unique word in test names only once (which of course is impractical) I am not sure I can use it in place of 0050 without checking names of other tests first. > I definitely don't want to make anybody's life harder. But if the > numbers aren't unique and the text connected to them is, then it seems > like we should just use the text primarily. True in principle, but it is harder to come up with unique substring of text, knowing only that the whole string is unique. Thanks.
Junio C Hamano <gitster@pobox.com> writes: > Jeff King <peff@peff.net> writes: > >> My plan is that you'd be able to say: >> >> Meta/make --test=filesystem,update-ref >> >> for t0050 and t1400 respectively. Or more interesting things like >> "git-svn-*" to skip all of the t91xx tests (or "git-svn/*" if we split >> them by directories). > ... > The word "filesystem"? Unless we have a rule that we can use each > unique word in test names only once (which of course is impractical) > I am not sure I can use it in place of 0050 without checking names > of other tests first. > >> I definitely don't want to make anybody's life harder. But if the >> numbers aren't unique and the text connected to them is, then it seems >> like we should just use the text primarily. > > True in principle, but it is harder to come up with unique substring > of text, knowing only that the whole string is unique. I think there was a mismatch in assumptions and expectations. I think your "plan" was to use the whole name (what comes after \d+- in the current naming scheme, except for the .sh at the end), and I was assuming a glob with implicit * on both ends would be used, i.e. $ ls -d t/*update-ref* would give me 1404 (update-ref-errors) in addition to 1400 (update-ref). So if the rule is to always spell out the full name if I wanted uniqueness, then it would work. In the cited usecase of knowing which two failed the first time, I should have the full filename of them available. Thanks.
On Fri, Oct 16, 2020 at 09:16:57AM -0700, Junio C Hamano wrote: > > I'm not sure where he suggests worse names. I'd think if anything we'd > > have better names, because they'd be even more meaningful (if people > > start using them for test selectors). FWIW, I also grep like that when > > looking for scripts. > > I didn't mean Jonathan suggested worse names. Unlike "I don't tend > to discover test scripts based on their filename", which was what > Jonathan said, I do look for tests based on their filename, so > having a good name matters (on the other hand, if you are the kind > of person who does not look for them by name, the naming may not > matter to you). Ah, I misunderstood. Thanks for clarifying (and I am very much in your camp that the names are useful). > As long as it is known that "filesystem" and "update-ref" can serve > as tokens to uniquely identify these two tests, it would be fine for > my purpose. But 0050 (under the rule that numbering must be unique) > would give me such an assurance much better without having to look > at any other test file. > > The word "filesystem"? Unless we have a rule that we can use each > unique word in test names only once (which of course is impractical) > I am not sure I can use it in place of 0050 without checking names > of other tests first. With your follow-up response: > So if the rule is to always spell out the full name if I wanted > uniqueness, then it would work. I think we are on the same page, and my intent was to match full names. So now you get "t0050" from some failed-test output (prove, or just the output from make failing), and you copy it into the command-line to use with "--test". And instead you'd just copy the full text name. It's a little less convenient because t1234 is short enough that I'd type it, and I'd probably cut-and-paste the text name. But other than that, I'd expect the procedure to be the same. The substring matches added by Elijah's series make sense for individual test snippets within a script, I think. And I think we could even add script-name matching now[1], without getting rid of the numbers. But if we do so, we should be careful to introduce it as an anchored match and not a substring match, to avoid having to switch it later. -Peff [1] And by "now" I don't mean we should hold up Elijah's patches for this feature, but that anybody who wishes to build it on top is free to do so without us having to make a decision on ditching the numbers entirely.
Jeff King <peff@peff.net> writes: > I think we are on the same page, and my intent was to match full names. > > So now you get "t0050" from some failed-test output (prove, or just the > output from make failing), and you copy it into the command-line to use > with "--test". And instead you'd just copy the full text name. It's a > little less convenient because t1234 is short enough that I'd type it, > and I'd probably cut-and-paste the text name. But other than that, I'd > expect the procedure to be the same. Most often, they are no longer in my scroll buffer, though. Two 4-digit integers I can easily hold in my head and type. Full names of two test scripts? That's a bit taxing. So, not really.
diff --git a/t/README b/t/README index 2adaf7c2d2..5fd8eaf595 100644 --- a/t/README +++ b/t/README @@ -258,16 +258,13 @@ 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. +The argument for --run, <test-selector>, is a list of description +substrings or regexes or individual test numbers or ranges with an +optional negation prefix (of '!') that define what tests in a test +suite to include (or exclude, if negated) 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. If --run starts with an unprefixed number or range the initial set of tests to run is empty. If the first item starts with '!' @@ -317,6 +314,18 @@ test in the test suite except from 7 up to 11: $ sh ./t9200-git-cvsexport-commit.sh --run='!7-11' +Sometimes there may be multiple tests with e.g. "setup" in their name +that are needed and rather than figuring out the number for all of them +we can just use "setup" as a substring/regex to match against the test +description: + + $ sh ./t0050-filesystem.sh --run=setup,9-11 + +or one could select both the setup tests and the rename ones (assuming all +relevant tests had those words in their descriptions): + + $ sh ./t0050-filesystem.sh --run=setup,rename + 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 diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh index 923281af93..07105b2078 100755 --- a/t/t0000-basic.sh +++ b/t/t0000-basic.sh @@ -705,7 +705,31 @@ test_expect_success '--run empty selectors' " EOF " -test_expect_success '--run invalid range start' " +test_expect_success '--run substring selector' " + run_sub_test_lib_test run-substring-selector \ + '--run empty selectors' \ + --run='relevant' <<-\\EOF && + test_expect_success \"relevant test\" 'true' + for i in 1 2 3 4 5 6 + do + test_expect_success \"other test #\$i\" 'true' + done + test_done + EOF + check_sub_test_lib_test run-substring-selector <<-\\EOF + > ok 1 - relevant test + > ok 2 # skip other test #1 (--run) + > ok 3 # skip other test #2 (--run) + > ok 4 # skip other test #3 (--run) + > ok 5 # skip other test #4 (--run) + > ok 6 # skip other test #5 (--run) + > ok 7 # skip other test #6 (--run) + > # passed all 7 test(s) + > 1..7 + EOF +" + +test_expect_success '--run keyword selection' " run_sub_test_lib_test_err run-inv-range-start \ '--run invalid range start' \ --run='a-5' <<-\\EOF && @@ -735,21 +759,6 @@ test_expect_success '--run invalid range end' " 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: invalid non-numeric in test selector: '1?' - EOF_ERR -" - test_set_prereq HAVEIT haveit=no diff --git a/t/test-lib.sh b/t/test-lib.sh index ef31f40037..2aca398e1e 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -769,6 +769,8 @@ match_pattern_list () { } match_test_selector_list () { + operation="$1" + shift title="$1" shift arg="$1" @@ -805,13 +807,13 @@ match_test_selector_list () { *-*) if expr "z${selector%%-*}" : "z[0-9]*[^0-9]" >/dev/null then - echo "error: $title: invalid non-numeric in range" \ + echo "error: $operation: invalid non-numeric in range" \ "start: '$orig_selector'" >&2 exit 1 fi if expr "z${selector#*-}" : "z[0-9]*[^0-9]" >/dev/null then - echo "error: $title: invalid non-numeric in range" \ + echo "error: $operation: invalid non-numeric in range" \ "end: '$orig_selector'" >&2 exit 1 fi @@ -819,9 +821,8 @@ match_test_selector_list () { *) if expr "z$selector" : "z[0-9]*[^0-9]" >/dev/null then - echo "error: $title: invalid non-numeric in test" \ - "selector: '$orig_selector'" >&2 - exit 1 + echo "$title" | grep -q "$selector" && return + continue fi esac @@ -1031,7 +1032,7 @@ test_skip () { skipped_reason="GIT_SKIP_TESTS" fi if test -z "$to_skip" && test -n "$run_list" && - ! match_test_selector_list '--run' $test_count "$run_list" + ! match_test_selector_list '--run' "$1" $test_count "$run_list" then to_skip=t skipped_reason="--run"