git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [ANNOUNCE] git-test: run automated tests against a range of Git commits
@ 2017-01-06 15:52 Michael Haggerty
  2017-01-07  7:18 ` Jeff King
  0 siblings, 1 reply; 5+ messages in thread
From: Michael Haggerty @ 2017-01-06 15:52 UTC (permalink / raw)
  To: git discussion list

I just released ⁠⁠⁠⁠`git test⁠⁠⁠⁠`, a script for running automated
tests across a range of Git commits and keeping track of the results in
git notes:

    https://github.com/mhagger/git-test

This is a script that I've been using in one form or another for years
and I find it really handy [1].

`git-test` is meant for people who want *all* of their commits (not just
the branch tip) to pass their automated tests.

Tl;dr: How to use `git test`

1.  Define the test you want to run. The string can be any shell
    command:

        git test add "make -j8 && make -j16 test"

2.  Create a separate linked worktree in which to run your tests:

        git worktree add --detach ../test HEAD

3.  Create a terminal window and `cd` to the directory containing
    the testing worktree, and run the test against all of the commits
    on your branch:

        cd ../test
        git test range master..mybranch

    If any of the commits are broken, `git tree` will display the
    error then stop with that commit checked out.

4.  As you work, whenever you want to test new commits, go to the
    testing terminal window and run the same command again:

        git test range master..mybranch

    `git test` is smart enough to remember which commits (actually,
    trees) have already been tested and only run the test against
    commits that have been changed or added. And since the tests are
    run in a different worktree, you can continue working in your
    main working directory while the tests run.

It is also possible to define more than one test suite in a given
repository, retry tests, etc. Type `git test help` or read the `README`
file [2] for more information.

`git test` stores the test results in git notes (under
`refs/notes/test/<name>`), linked to the commit's tree SHA-1. This means
that test results remain valid even across some kinds of commit
rewriting, like changes to commit metadata or squashing adjacent
commits, and a subset of results even remains valid if a commit is split
or if some commits earlier in a patch series are reordered.

I don't plan to turn this into a gigantic project or anything, but I
find this script really useful so I wanted to put it out in the world.
Feedback and/or pull requests welcome!

Michael

[1] The name sucks, I know :-/
[2] https://github.com/mhagger/git-test/blob/master/README.md


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [ANNOUNCE] git-test: run automated tests against a range of Git commits
  2017-01-06 15:52 [ANNOUNCE] git-test: run automated tests against a range of Git commits Michael Haggerty
@ 2017-01-07  7:18 ` Jeff King
  2017-01-08  9:52   ` Michael Haggerty
  0 siblings, 1 reply; 5+ messages in thread
From: Jeff King @ 2017-01-07  7:18 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: git discussion list

On Fri, Jan 06, 2017 at 04:52:16PM +0100, Michael Haggerty wrote:

> I just released ⁠⁠⁠⁠`git test⁠⁠⁠⁠`, a script for running automated
> tests across a range of Git commits and keeping track of the results in
> git notes:
> 
>     https://github.com/mhagger/git-test
> 
> This is a script that I've been using in one form or another for years
> and I find it really handy [1].

Neat. I usually "git rebase -x 'make -j8 test' @{u}" after finishing a
topic to make sure the intermediate steps are good. But it would be neat
to have this running continuously in the background to alert me to
problems sooner (and the key thing there is that it remembers
already-run tests, so it should be safe to basically for new commits
every 10 seconds or so).

I did hit a few interesting cases trying out "git test". So here's a
narrative, and you can pick out where there may be room for improvement
in the tool, and where I'm just being dumb. :)

I tried it out first on a topic I finished earlier today, which has 3
commits. So I did:

  $ git test add 'make -j8 test'
  $ git test range @{u}..HEAD

It barfed on the first commit, because the script expects "git co" to
work, but I don't have that alias. No big deal (and I already submitted
a PR to fix it).

So then I reinvoked it like:

  $ git test range @{u}..HEAD

and it actually ran some tests. Yay.

And then of course I wanted to prove to myself how cool the notes
feature is, so I ran it again. It didn't run any tests this time. Yay
again. But there were a few surprises:

  $ git test range @{u}..HEAD
  setup_test default
  Using test default; command: make -j8 test
  Old status: bad
  Tree 9fcdbd5c78^{tree} is already known to be bad!
  Old status: good
  Tree c22f4f6624^{tree} is already known to be good.
  Old status: good
  Tree 19e2e62e5e^{tree} is already known to be good.
  Already on 'jk/wait-for-child-cleanup'
  Your branch is ahead of 'origin/master' by 3 commits.

  ALL TESTS SUCCESSFUL

My initial run with "git co" had left the first commit marked as "bad".
That's not _too_ surprising, since it did indeed fail. I think it's
probably a bug to record a failure note, though, if checking out fails.
It's not necessarily an immutable property of the tree. In my case, it
was obviously dependent on a change in the git-test code, but that's
hopefully a fairly uncommon occurrence. But there are other reasons for
git-checkout to complain. For instance, imagine topic "foo" creates file
"foo.c". If I do:

  $ echo content >foo.c
  $ git test foo@{u}..foo

then checkout will complain about overwriting the untracked "foo.c".
It's reasonable to abort the operation there, but probably not to write
a permanent failure note. The problem wasn't in "foo", but in my local
tree.

The second thing that surprised me was "ALL TESTS SUCCESSFUL", when
clearly one of them was known-bad. :)

So at this point I knew I needed to re-run the test. Looks like there's
a "--force" option. Let's try it. There's no need to re-run the other
two, so let's just give it one commit:

  $ git test range -f HEAD~2
  ...
  Object 95649d6cf9ec68f05d1dc57ec1b989b8d263a7ae^{tree} has no note
  Object e1970ce43abfbf625bce68516857e910748e5965^{tree} has no note
  Object 368f99d57e8ed17243f2e164431449d48bfca2fb^{tree} has no note
  Object ceede59ea90cebad52ba9c8263fef3fb6ef17593^{tree} has no note
  Object dfe070511c652f2b8e1bf6540f238c9ca9ba41d3^{tree} has no note
  Object 902d960b382a0cd424618ff4e1316da40e4be2f6^{tree} has no note
  ...

This started spewing out many lines like the one above, until I hit ^C.
Yikes!

Thinking something was wrong with the "-f" option, I tried it without:

  $ git test range -f HEAD~2
  ...
  commit e83c5163316f89bfbde7d9ab23ca2e25604af290 (origin/initial)
  Author: Linus Torvalds <torvalds@ppc970.osdl.org>
  Date:   Thu Apr 7 15:13:13 2005 -0700

      Initial revision of "git", the information manager from hell

Oops. Now I see the problem. I was expecting the arguments to be
rebase-like. But they're really rev-list like. So in both cases it
wanted to test all the way up from the root commits to HEAD~2.  The "-f"
version never got to testing a commit because it was so busy trying to
delete the old notes.

I see the symmetry and simplicity in allowing the user to specify a full
range. But it also seems like it's easy to make a mistake that's going
to want to test a lot of commits. I wonder if it should complain when
there's no lower bound to the commit range. Or alternatively, if there's
a single positive reference, treat it as a lower bound, with HEAD as the
upper bound (which is vaguely rebase-like).

A few other observations about the note deletion:

  - The "has no note" message should perhaps be suppressed. We're just
    trying to overwrite the value if there is one (alternatively,
    instead of removing it, just overwrite it, so the old note stays
    until we get a result one way or the other).

  - It was sufficiently slow that it looks like we invoke "git notes
    remove" once per commit. It would be a lot more efficient to batch
    them (not just in terms of process startup, but because you're going
    to write a _ton_ of intermediate notes trees).

    Of course none of that matters if you don't do something stupid like
    trying to "git test" 45,000 commits. :)

So OK. Now I know what's going on. And I can get what I want with:

  $ git test range -f HEAD~3..HEAD~2

which works, and now all of my tests are correctly marked. Of course
that's a lot to type. It would be easier as:

  $ git test range -f -1 HEAD~2
  usage: git-test [-h] {add,range,help} ...
  git-test: error: unrecognized arguments: HEAD~2

but the tool doesn't seem to like passing through the rev-list argument.

It would be even easier if I could just repeat my range and only re-test
the "bad" commits. It was then that I decided to actually read the rest
of "git test help range" and see that you already wrote such an option,
cleverly hidden under the name "--retest".

And one final nit. I notice there is also a "--keep-going" option. Which
made me surprised that we bothered to test HEAD~1 and HEAD, when we knew
that HEAD~2 was bogus. I suspect this is related to the "ALL TESTS
SUCCESSFUL" issue. Presumably cached test results are not treated as
"bad" in general, which seems funny to me.

So those were all little cosmetic things. The other big thing I wanted
to see was what it's like to fix a bug deep in a topic. So I used "git
rebase -i" to inset a compile error into the first commit of my 3-patch
series. And then I tested it:

  $ git test add -t compile 'make -j8'
  $ git test range -t compile HEAD~3..

As predicted, it stopped at the first commit and told me it was buggy.
But I'm dumped onto a detached HEAD, and I'm on my own to actually get
the working tree to a state where I can test and fix on my actual
branch.

That's a nice outcome of the "git rebase -x make" approach. When you hit
a bug, you can fix it, "git commit --amend", and "git rebase
--continue". The downside is that it takes ownership of the branch while
you're doing it. So the whole concept of "run this in the background
with a separate worktree" breaks down completely.

I think it should be possible to script the next steps, though.
Something like like "git test fix foo", which would:

  - expand the range of foo@{u}..foo to get the list of commits

  - see which ones were marked as broken

  - kick off an interactive rebase, but override GIT_EDITOR to mark any
    broken ones as "edit" instead of "pick"

That lets you separate the act of testing from the act of fixing. You
can let the tester run continuously in the background, and only stop to
fix when you're at an appropriate point in your work.

Anyway. Seems like a neat tool. I may play around with it and see if I
can fit it into my workflow.

-Peff

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [ANNOUNCE] git-test: run automated tests against a range of Git commits
  2017-01-07  7:18 ` Jeff King
@ 2017-01-08  9:52   ` Michael Haggerty
  2017-01-10  9:35     ` Jeff King
  0 siblings, 1 reply; 5+ messages in thread
From: Michael Haggerty @ 2017-01-08  9:52 UTC (permalink / raw)
  To: Jeff King; +Cc: git discussion list

On 01/07/2017 08:18 AM, Jeff King wrote:
> On Fri, Jan 06, 2017 at 04:52:16PM +0100, Michael Haggerty wrote:
> 
>> I just released ⁠⁠⁠⁠`git test⁠⁠⁠⁠`, a script for running automated
>> tests across a range of Git commits and keeping track of the results in
>> git notes:
>>
>>     https://github.com/mhagger/git-test
>>
>> This is a script that I've been using in one form or another for years
>> and I find it really handy [1].
> 
> Neat. I usually "git rebase -x 'make -j8 test' @{u}" after finishing a
> topic to make sure the intermediate steps are good. But it would be neat
> to have this running continuously in the background to alert me to
> problems sooner (and the key thing there is that it remembers
> already-run tests, so it should be safe to basically for new commits
> every 10 seconds or so).
> 
> I did hit a few interesting cases trying out "git test". So here's a
> narrative, and you can pick out where there may be room for improvement
> in the tool, and where I'm just being dumb. :)
> 
> I tried it out first on a topic I finished earlier today, which has 3
> commits. So I did:
> 
>   $ git test add 'make -j8 test'
>   $ git test range @{u}..HEAD
> 
> It barfed on the first commit, because the script expects "git co" to
> work, but I don't have that alias. No big deal (and I already submitted
> a PR to fix it).

I make the same mistake in most of my scripts :-/ Thanks for the PR; I
merged it.

> So then I reinvoked it like:
> 
>   $ git test range @{u}..HEAD
> 
> and it actually ran some tests. Yay.
> 
> And then of course I wanted to prove to myself how cool the notes
> feature is, so I ran it again. It didn't run any tests this time. Yay
> again. But there were a few surprises:
> 
>   $ git test range @{u}..HEAD
>   setup_test default
>   Using test default; command: make -j8 test
>   Old status: bad
>   Tree 9fcdbd5c78^{tree} is already known to be bad!
>   Old status: good
>   Tree c22f4f6624^{tree} is already known to be good.
>   Old status: good
>   Tree 19e2e62e5e^{tree} is already known to be good.
>   Already on 'jk/wait-for-child-cleanup'
>   Your branch is ahead of 'origin/master' by 3 commits.
> 
>   ALL TESTS SUCCESSFUL
>
> My initial run with "git co" had left the first commit marked as "bad".
> That's not _too_ surprising, since it did indeed fail. I think it's
> probably a bug to record a failure note, though, if checking out fails.
> It's not necessarily an immutable property of the tree.

I definitely agree. This was an oversight which I just fixed.

> [...]
> 
> The second thing that surprised me was "ALL TESTS SUCCESSFUL", when
> clearly one of them was known-bad. :)

Replayed results weren't being treated internally as failures. That's
fixed, too.

> So at this point I knew I needed to re-run the test. Looks like there's
> a "--force" option. Let's try it. There's no need to re-run the other
> two, so let's just give it one commit:
> 
>   $ git test range -f HEAD~2
>   ...
>   Object 95649d6cf9ec68f05d1dc57ec1b989b8d263a7ae^{tree} has no note
>   Object e1970ce43abfbf625bce68516857e910748e5965^{tree} has no note
>   Object 368f99d57e8ed17243f2e164431449d48bfca2fb^{tree} has no note
>   Object ceede59ea90cebad52ba9c8263fef3fb6ef17593^{tree} has no note
>   Object dfe070511c652f2b8e1bf6540f238c9ca9ba41d3^{tree} has no note
>   Object 902d960b382a0cd424618ff4e1316da40e4be2f6^{tree} has no note
>   ...
> 
> This started spewing out many lines like the one above, until I hit ^C.
> Yikes!
> 
> [...]
> I see the symmetry and simplicity in allowing the user to specify a full
> range. But it also seems like it's easy to make a mistake that's going
> to want to test a lot of commits. I wonder if it should complain when
> there's no lower bound to the commit range. Or alternatively, if there's
> a single positive reference, treat it as a lower bound, with HEAD as the
> upper bound (which is vaguely rebase-like).

I see how this might be unexpected, and it's definitely inconvenient at
some times (like when you want to test a single commit). I thought it
would be nice to allow arbitrary `rev-list` expressions (albeit
currently only a single word), but I think that you are right that other
semantics would be more convenient.

I'm thinking of maybe

* If an argument matches `*..*`, pass it to `rev-list` (like now).

* Otherwise, treat each argument as a single commit/tree (i.e., pass it
to `rev-parse`).

* If no argument is specified, test `@{u}..` (assuming that an
  upstream is configured). Though actually, this won't be as
  convenient as it sounds, because (a) `git test` is often run
  in a separate worktree, and (2) on errors, it currently leaves the
  repository with a detached `HEAD`.

* Support a `--stdin` option, to read a list of commits/trees to test
  from standard input. By this mechanism, users could use arbitrary
  `rev-list` commands to choose what to test.

> A few other observations about the note deletion:
> 
>   - The "has no note" message should perhaps be suppressed. We're just
>     trying to overwrite the value if there is one (alternatively,
>     instead of removing it, just overwrite it, so the old note stays
>     until we get a result one way or the other).

Yes. That's a one-time problem that I haven't seen in a long time. The
script is overly chatty in general.

Aside: It would be nice if `git notes` had a subcommand to initialize a
note reference with an empty tree. (I know how to do it longhand, but
it's awkward and it should be possible to do it via the `notes` interface.)

I think ideally `git notes add` would look for pre-existing notes, and:

* If none are found, create an empty notes reference.

* If pre-existing notes are found and there was no existing test with
  that name, probably just leave the old notes in place.

* If pre-existing notes are found and there was already a test with
  that name but a different command, perhaps insist that the user
  decide explicitly whether to forget the old results or continue using
  them. This might help users avoid the mistake of re-using old results
  even if they change the manner of testing.

>   - It was sufficiently slow that it looks like we invoke "git notes
>     remove" once per commit. It would be a lot more efficient to batch
>     them (not just in terms of process startup, but because you're going
>     to write a _ton_ of intermediate notes trees).
> 
>     Of course none of that matters if you don't do something stupid like
>     trying to "git test" 45,000 commits. :)

Yeah, I've never experienced that problem myself :-P But I see that
notes supports `git notes remove --ignore-missing --stdin`, so that will
be easy to implement.

> [...]
> It would be even easier if I could just repeat my range and only re-test
> the "bad" commits. It was then that I decided to actually read the rest
> of "git test help range" and see that you already wrote such an option,
> cleverly hidden under the name "--retest".

I think you were being ironic, but if not, would this have been easier
to find under another name?

> And one final nit. I notice there is also a "--keep-going" option. Which
> made me surprised that we bothered to test HEAD~1 and HEAD, when we knew
> that HEAD~2 was bogus. I suspect this is related to the "ALL TESTS
> SUCCESSFUL" issue.

Yes, that's part of the same bug from above.

> So those were all little cosmetic things. The other big thing I wanted
> to see was what it's like to fix a bug deep in a topic. So I used "git
> rebase -i" to inset a compile error into the first commit of my 3-patch
> series. And then I tested it:
> 
>   $ git test add -t compile 'make -j8'
>   $ git test range -t compile HEAD~3..
> 
> As predicted, it stopped at the first commit and told me it was buggy.
> But I'm dumped onto a detached HEAD, and I'm on my own to actually get
> the working tree to a state where I can test and fix on my actual
> branch.

Yeah, this is awkward, not only because many people don't know what to
make of detached HEAD, but also because it makes it awkward in general
to use `git test` in your main working directory. I didn't model this
behavior on `git rebase --interactive`'s `edit` command, because I
rarely use that. But I can see how they would fit together pretty well
for people who like that workflow.

I've considered that rather than leave you in a detached HEAD state,
maybe `git test` should always restore your old branch. But it seems
like it would more often be useful to be in the directory with the
broken commit checked out and any test results, coredumps, etc intact.

I would definitely like to implement a `git test reset` command that
returns you to your initial branch (like `git bisect reset`).

I like your idea of a `git test fix` command:

> [...]
> I think it should be possible to script the next steps, though.
> Something like like "git test fix foo", which would:
> 
>   - expand the range of foo@{u}..foo to get the list of commits
> 
>   - see which ones were marked as broken
> 
>   - kick off an interactive rebase, but override GIT_EDITOR to mark any
>     broken ones as "edit" instead of "pick"
> 
> That lets you separate the act of testing from the act of fixing. You
> can let the tester run continuously in the background, and only stop to
> fix when you're at an appropriate point in your work.

I think you would usually only want to mark only the *first* broken
commit as "edit", because often errors cascade to descendant commits.

Thanks for all the great feedback!

Michael


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [ANNOUNCE] git-test: run automated tests against a range of Git commits
  2017-01-08  9:52   ` Michael Haggerty
@ 2017-01-10  9:35     ` Jeff King
  2017-01-12  5:02       ` Michael Haggerty
  0 siblings, 1 reply; 5+ messages in thread
From: Jeff King @ 2017-01-10  9:35 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: git discussion list

On Sun, Jan 08, 2017 at 10:52:25AM +0100, Michael Haggerty wrote:

> > I see the symmetry and simplicity in allowing the user to specify a full
> > range. But it also seems like it's easy to make a mistake that's going
> > to want to test a lot of commits. I wonder if it should complain when
> > there's no lower bound to the commit range. Or alternatively, if there's
> > a single positive reference, treat it as a lower bound, with HEAD as the
> > upper bound (which is vaguely rebase-like).
> 
> I see how this might be unexpected, and it's definitely inconvenient at
> some times (like when you want to test a single commit). I thought it
> would be nice to allow arbitrary `rev-list` expressions (albeit
> currently only a single word), but I think that you are right that other
> semantics would be more convenient.
> 
> I'm thinking of maybe
> 
> * If an argument matches `*..*`, pass it to `rev-list` (like now).
> 
> * Otherwise, treat each argument as a single commit/tree (i.e., pass it
> to `rev-parse`).
> 
> * If no argument is specified, test `@{u}..` (assuming that an
>   upstream is configured). Though actually, this won't be as
>   convenient as it sounds, because (a) `git test` is often run
>   in a separate worktree, and (2) on errors, it currently leaves the
>   repository with a detached `HEAD`.
> 
> * Support a `--stdin` option, to read a list of commits/trees to test
>   from standard input. By this mechanism, users could use arbitrary
>   `rev-list` commands to choose what to test.

That seems quite reasonable to me. The rebase semantics of "a single
argument is my upstream" is convenient, but I think it also confuses
people. Yours seem very simple to explain.

It doesn't allow complex stuff like "git test ^foo ^bar HEAD", but I
don't think "git rev-list ^foo ^bar HEAD | git test --stdin" is really
so bad for complicated cases like that (if anybody would even ever want
such a thing).

> Aside: It would be nice if `git notes` had a subcommand to initialize a
> note reference with an empty tree. (I know how to do it longhand, but
> it's awkward and it should be possible to do it via the `notes` interface.)
> 
> I think ideally `git notes add` would look for pre-existing notes, and:
> 
> * If none are found, create an empty notes reference.
> 
> * If pre-existing notes are found and there was no existing test with
>   that name, probably just leave the old notes in place.
> 
> * If pre-existing notes are found and there was already a test with
>   that name but a different command, perhaps insist that the user
>   decide explicitly whether to forget the old results or continue using
>   them. This might help users avoid the mistake of re-using old results
>   even if they change the manner of testing.

I'm not quite sure what you mean here. By "test" and "command", do you
mean the test name that is used in the notes ref, and the command that
it is defined as?

In the notes-cache.c subsystem, the commit message stores a validity
token which must match in order to use the cache. You could do something
similar here (store the executed command in the commit message, and
invalidate the cache the user has changed the command). The notes-cache
stuff isn't available outside of the C code, though. You could either
expose it, or just do something similar longhand.

Thinking about it, though, I did notice that the tree sha1 is not the
only input to the cache. Things like config.mak (not to mention the
system itself) contribute to the test results. So no system will ever be
perfect, and it seems like just making an easy way to force a retest (or
just invalidate the whole cache) would be sufficient.

But maybe I totally missed what you were trying to say here.

> > It would be even easier if I could just repeat my range and only re-test
> > the "bad" commits. It was then that I decided to actually read the rest
> > of "git test help range" and see that you already wrote such an option,
> > cleverly hidden under the name "--retest".
> 
> I think you were being ironic, but if not, would this have been easier
> to find under another name?

Sorry, the knob on my sarcasm module must have accidentally been knocked
down from "so thick it's obvious even by email".

Yes, I did just mean that I was being blind. You not only had added the
option already, but gave it the exact name I was about to propose it
under. :)

> Yeah, this is awkward, not only because many people don't know what to
> make of detached HEAD, but also because it makes it awkward in general
> to use `git test` in your main working directory. I didn't model this
> behavior on `git rebase --interactive`'s `edit` command, because I
> rarely use that. But I can see how they would fit together pretty well
> for people who like that workflow.

Yeah, after sleeping on it, I think it's best if "git test" remains
separate from that. It's primary function is to run the test, possibly
serving up a cached answer. So it would be perfectly reasonable to do:

  git rebase -x 'git test range HEAD'

to accomplish the interactive testing (though perhaps just "git test"
would be a nice synonym for that).

And then "jump to a thing that I know is broken" becomes a separate
action, whether you are using "git test" or not. I wonder if we could
have:

  git rebase -e HEAD~2

to do an interactive rebase, with "edit" for HEAD~2. I feel like
somebody even proposed that at one point, but I don't think it got
merged.

And then "git test fix" basically becomes:

  git rebase -e "$(git test first-broken)"

though I think you'd still want a shorthand for that.

> > I think it should be possible to script the next steps, though.
> > Something like like "git test fix foo", which would:
> > 
> >   - expand the range of foo@{u}..foo to get the list of commits
> > 
> >   - see which ones were marked as broken
> > 
> >   - kick off an interactive rebase, but override GIT_EDITOR to mark any
> >     broken ones as "edit" instead of "pick"
> > 
> > That lets you separate the act of testing from the act of fixing. You
> > can let the tester run continuously in the background, and only stop to
> > fix when you're at an appropriate point in your work.
> 
> I think you would usually only want to mark only the *first* broken
> commit as "edit", because often errors cascade to descendant commits.

Yeah, I had a similar thought. OTOH, if you didn't say "--keep-going",
you'd only have one breakage either way. I doubt it matters that much in
practice, and either behavior would probably be fine until somebody
comes up with a good use for the multi-commit case.

-Peff

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [ANNOUNCE] git-test: run automated tests against a range of Git commits
  2017-01-10  9:35     ` Jeff King
@ 2017-01-12  5:02       ` Michael Haggerty
  0 siblings, 0 replies; 5+ messages in thread
From: Michael Haggerty @ 2017-01-12  5:02 UTC (permalink / raw)
  To: Jeff King; +Cc: git discussion list

On 01/10/2017 10:35 AM, Jeff King wrote:
> On Sun, Jan 08, 2017 at 10:52:25AM +0100, Michael Haggerty wrote:
>> [...]

Since my last email, I have implemented a bunch of what we discussed
[1]. Because of the new semantics, I also *renamed the main command* from

    git test range [...]

to

    git test run [...]

>> I'm thinking of maybe
>>
>> * If an argument matches `*..*`, pass it to `rev-list` (like now).

This was already implemented.

>> * Otherwise, treat each argument as a single commit/tree (i.e., pass it
>> to `rev-parse`).

This is now implemented, too. Plus, it is now allowed to specify
multiple commits and/or ranges in a single invocation.

>> * If no argument is specified, test `@{u}..` (assuming that an
>>   upstream is configured). Though actually, this won't be as
>>   convenient as it sounds, because (a) `git test` is often run
>>   in a separate worktree, and (2) on errors, it currently leaves the
>>   repository with a detached `HEAD`.

I decided that if no argument is specified, it makes more sense to test
HEAD if the working tree is clean, and the contents of the working tree
otherwise. In the latter case no results are stored to notes, but it is
still useful to be able to run a preconfigured test quickly while
working. These are both implemented.

>> * Support a `--stdin` option, to read a list of commits/trees to test
>>   from standard input. By this mechanism, users could use arbitrary
>>   `rev-list` commands to choose what to test.

This is now implemented, too.

> [...]
>> I think ideally `git notes add` would look for pre-existing notes, and:
>>
>> * If none are found, create an empty notes reference.
>>
>> * If pre-existing notes are found and there was no existing test with
>>   that name, probably just leave the old notes in place.
>>
>> * If pre-existing notes are found and there was already a test with
>>   that name but a different command, perhaps insist that the user
>>   decide explicitly whether to forget the old results or continue using
>>   them. This might help users avoid the mistake of re-using old results
>>   even if they change the manner of testing.
> 
> I'm not quite sure what you mean here. By "test" and "command", do you
> mean the test name that is used in the notes ref, and the command that
> it is defined as?

By "test", I mean a test as configured in `git-test` and referred to by
its name (e.g., in `--test=name`). Currently the only configuration for
a test is `test.<name>.command`, but I structured the namespace that way
to leave room to add more configuration in the future.

> In the notes-cache.c subsystem, the commit message stores a validity
> token which must match in order to use the cache. You could do something
> similar here (store the executed command in the commit message, and
> invalidate the cache the user has changed the command). The notes-cache
> stuff isn't available outside of the C code, though. You could either
> expose it, or just do something similar longhand.
> 
> Thinking about it, though, I did notice that the tree sha1 is not the
> only input to the cache. Things like config.mak (not to mention the
> system itself) contribute to the test results. So no system will ever be
> perfect, and it seems like just making an easy way to force a retest (or
> just invalidate the whole cache) would be sufficient.

It's true that the tree and the test command are not the only inputs to
the testing machinery. I wasn't hoping to build a watertight system to
prevent accidental use of old results when `config.mak` or something
else in the environment has changed. I was only trying to give the user
a reminder that if they change the command, it's a good time to consider
whether the old results have to be invalidated.

I suppose if we wanted to make this system more watertight, we could let
the test configuration specify the names of other files on which its
results depend; for example,

    test.default.command = make -j16 test
    test.default.auxiliaryInput = config.mak

and include some kind of hash of the auxiliary inputs in a validity
token. But that feels like overkill.

> [...]
>> Yeah, this is awkward, not only because many people don't know what to
>> make of detached HEAD, but also because it makes it awkward in general
>> to use `git test` in your main working directory. I didn't model this
>> behavior on `git rebase --interactive`'s `edit` command, because I
>> rarely use that. But I can see how they would fit together pretty well
>> for people who like that workflow.
> 
> Yeah, after sleeping on it, I think it's best if "git test" remains
> separate from that. It's primary function is to run the test, possibly
> serving up a cached answer. So it would be perfectly reasonable to do:
> 
>   git rebase -x 'git test range HEAD'
> 
> to accomplish the interactive testing (though perhaps just "git test"
> would be a nice synonym for that).

That invocation can now be written `git test run`, which is a bit
shorter. There could be a special case that `git test` is shorthand for
`git test run`, but it quickly gets ambiguous if the user wants to start
adding any `run` arguments.

> And then "jump to a thing that I know is broken" becomes a separate
> action, whether you are using "git test" or not. I wonder if we could
> have:
> 
>   git rebase -e HEAD~2
> 
> to do an interactive rebase, with "edit" for HEAD~2. I feel like
> somebody even proposed that at one point, but I don't think it got
> merged.

That sounds handy. (The GIT_EDITOR thing is pretty hacky.)

> And then "git test fix" basically becomes:
> 
>   git rebase -e "$(git test first-broken)"
> 
> though I think you'd still want a shorthand for that.

There could be a `git test fix --rebase <range>` that does this. Plus my
preference, `git test fix --branch=fix <range>`, which creates a branch
named `fix` at the first broken commit and checks it out. (If the fix is
nontrivial, my next step is usually to `git imerge rebase` the original
branch onto the `fix` branch.)

>>> I think it should be possible to script the next steps, though.
>>> Something like like "git test fix foo", which would: [...]
>>
>> I think you would usually only want to mark only the *first* broken
>> commit as "edit", because often errors cascade to descendant commits.
> 
> Yeah, I had a similar thought. OTOH, if you didn't say "--keep-going",
> you'd only have one breakage either way. [...]

True enough.

Thanks for all your feedback!

Michael

[1] https://github.com/mhagger/git-test


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-01-12  5:04 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-06 15:52 [ANNOUNCE] git-test: run automated tests against a range of Git commits Michael Haggerty
2017-01-07  7:18 ` Jeff King
2017-01-08  9:52   ` Michael Haggerty
2017-01-10  9:35     ` Jeff King
2017-01-12  5:02       ` Michael Haggerty

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).