Re: [PATCHv2] branch -d: test if we can delete broken refs
Aside from one tiny formatting nit (see below), the test looks good to me. On the other hand, this is kind of an aspirational test; I don't know that the tested functionality has ever worked or that anybody has ever claimed that it works. So my feeling is that the addition of the test would feel more natural in the patch series that implements the new feature. But I don't feel strongly about it. Michael On 11/26/2014 01:59 AM, Stefan Beller wrote: From: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Stefan Beller sbel...@google.com --- Changes v1-v2 * relocated the test from t1402 to t3200 * reword the commit message title to fit in with similar commits touching t/t3200-branch.sh t/t3200-branch.sh | 8 1 file changed, 8 insertions(+) diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index 432921b..fa7d7bd 100755 --- a/t/t3200-branch.sh +++ b/t/t3200-branch.sh @@ -163,6 +163,14 @@ test_expect_success 'git branch --list -d t should fail' ' test_path_is_missing .git/refs/heads/t ' +test_expect_failure 'git branch -d can delete ref with broken sha1' ' + echo pointing nowhere .git/refs/heads/brokensha1 Please no space between the and the filename. + test_when_finished rm -f .git/refs/heads/brokensha1 + git branch -d brokensha1 + git branch output + ! grep -e brokensha1 output +' + test_expect_success 'git branch --column' ' COLUMNS=81 git branch --column=column actual cat expected \EOF -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- 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: [PATCHv2] branch -d: test if we can delete broken refs
Michael Haggerty mhag...@alum.mit.edu writes: ... On the other hand, this is kind of an aspirational test; I don't know that the tested functionality has ever worked or that anybody has ever claimed that it works. So my feeling is that the addition of the test would feel more natural in the patch series that implements the new feature. But I don't feel strongly about it. I share your feeling. In the aspiration followed by realization pattern, the realization commit shows a change in t/ hierarchy that turns test_expect_failure into test_expect_success and it is likely that what is being tested will fall outside the context. Unless the test title is phrased very well, it would not be obvious from the patch text to the t/ hierarchy alone what behaviour is being corrected when looking at the realization commit. If aspiration and realization are in a same series, that would not be a problem, but it is if the commits that add aspirational test and realization are too far apart. If it is pretty clear to everybody that another topic to realize the aspiration will be coming in a not so distant future, I think it is fine, though. -- 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
[PATCHv2] branch -d: test if we can delete broken refs
From: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Stefan Beller sbel...@google.com --- Changes v1-v2 * relocated the test from t1402 to t3200 * reword the commit message title to fit in with similar commits touching t/t3200-branch.sh t/t3200-branch.sh | 8 1 file changed, 8 insertions(+) diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index 432921b..fa7d7bd 100755 --- a/t/t3200-branch.sh +++ b/t/t3200-branch.sh @@ -163,6 +163,14 @@ test_expect_success 'git branch --list -d t should fail' ' test_path_is_missing .git/refs/heads/t ' +test_expect_failure 'git branch -d can delete ref with broken sha1' ' + echo pointing nowhere .git/refs/heads/brokensha1 + test_when_finished rm -f .git/refs/heads/brokensha1 + git branch -d brokensha1 + git branch output + ! grep -e brokensha1 output +' + test_expect_success 'git branch --column' ' COLUMNS=81 git branch --column=column actual cat expected \EOF -- 2.2.0.rc3 -- 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
