Opened at 2013-05-28T23:38:20Z
Closed at 2013-08-31T17:06:11Z
#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: |
Description
<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.
[later...]
<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
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: ↓ 5 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

The implementation in the pull request will print this message if the current version is "dirty":
This is dependent on the version having been updated. It will always work as intended if you run tests via one the following commands:
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.)