#1992 closed defect (fixed)

warn if testing uncommitted code

Reported by: daira Owned by: warner
Priority: normal Milestone: undecided
Component: dev-infrastructure Version: 1.10.0
Keywords: test git review-needed brians-opinion-needed Cc:
Launchpad Bug:


   <zooko>: I think you were advocating editing the current working directory
            to have a certain version in it, running tests, then committing all
            of the current working directory.
   <zooko>: I now see why (because of the tests).
   <zooko>: An alternative would be to commit a part of the current working set,
            such as by "git commit $THISFILE" or even the interactive hunk-picking
            features, then run tests on that committed version.
<nejucomo>: What I'm advocating against is using "git add" to select some set of
            the working directory, and then committing a state which you've never
            actually interacted with.
<nejucomo>: zooko: What if the $THISFILE imports a python module which is also
            altered, but which is not in the index?
<nejucomo>: Unit tests would pass, but if anyone checked out that commit, unit
            tests would fail.
  <zancas>: Huh...   why?
  <zancas>: 'git add' only makes new files be part of the index.
  <zancas>: When you commit, all previously tracked state is still tracked.
  <zancas>: The commit-tree that you get when you run 'git commit' will contain
            everything that was in the previous commit, plus the new blobs that
            track the files you added.
   <daira>: if the tree was dirty when you ran tests, I think nejucomo meant
   <daira>: maybe we could make the tests output a warning if the tree is dirty?
   <daira>: (that would be pretty easy to do)


Change History (7)

comment:1 Changed at 2013-05-29T00:12:34Z by daira

The implementation in the pull request will print this message if the current version is "dirty":

WARNING: the source tree has been modified since the last commit.
(It is usually preferable to commit, then test, then amend the commit(s)
if the tests fail.)

This is dependent on the version having been updated. It will always work as intended if you run tests via one the following commands:

  • python setup.py test
  • python setup.py trial
  • make test
  • make test-coverage
  • make check
  • make quicktest
  • make quicktest-coverage
  • make tmpfstest

but it won't work as intended if you run bin/tahoe debug trial without having built since the last source change or commit. (Note: that's a bad idea anyway and is not supported.)

comment:2 Changed at 2013-05-29T00:24:07Z by daira

  • Component changed from code to dev-infrastructure

comment:3 Changed at 2013-05-29T17:25:58Z by zooko

  • Owner changed from zooko to warner

I'd be curious what Brian thinks. I've been trying to follow his recommended git workflows.

comment:4 follow-up: Changed at 2013-05-29T17:28:10Z by gdt

I'm not Brian, but I think the notion that one prepares a commit and then tests it is sound. So the tests warning that the source tree is dirty is a good idea.

comment:5 in reply to: ↑ 4 Changed at 2013-05-29T21:52:10Z by zooko

Replying to gdt:

I'm not Brian, but I think the notion that one prepares a commit and then tests it is sound. So the tests warning that the source tree is dirty is a good idea.

Thanks, gdt! It sounds like a pretty good idea to me, too. I think a lot of people get misdirected by git's terminology of calling a version a "commit". There is no commitment involved. You can change your mind at any time. ☺ Now a git push, to a repository that other people can read, now that's commitment. Even then it might not be a big deal if you are pushing to a repo+branch which everyone knows can get clobbered at any time.

comment:6 Changed at 2013-06-02T15:47:38Z by daira

  • Keywords brians-opinion-needed added

Review still needed for the actual patch.

comment:7 Changed at 2013-08-31T17:06:11Z by Daira Hopwood <daira@…>

  • Resolution set to fixed
  • Status changed from new to closed

In aa29f2655db37fe8bd0bbf2a8c1c5ddccf24791c/trunk:

tahoe debug trial: print a warning message if testing uncommitted code. fixes #1992

Signed-off-by: Daira Hopwood <david-sarah@…>

Note: See TracTickets for help on using tickets.