Re: [PATCH v2 3/3] t9903: add extra tests for bash.showDirtyState
Martin Erik Werner martinerikwer...@gmail.com writes: Added 3 extra tests for the bash.showDirtyState config option, tests should now cover all combinations of the shell var being set/unset and the config option being enabled/disabled, given a dirty file. Strictly speaking, you have 6 not 4 combinations (shell variable set/unset * config missing/set to false/set to true). I think these additional tests cover should all 6 because config missing case should already have had tests before bash.showDirtyState was added. * Renamed test 'disabled by config' to 'shell variable set with config disabled' for consistency --- Sign-off? t/t9903-bash-prompt.sh | 38 +- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index cb008e2..fa81b09 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -360,12 +360,48 @@ test_expect_success 'prompt - dirty status indicator - before root commit' ' test_cmp expected $actual ' -test_expect_success 'prompt - dirty status indicator - disabled by config' ' +test_expect_success 'prompt - dirty status indicator - shell variable unset with config disabled' ' printf (master) expected echo dirty file test_when_finished git reset --hard test_config bash.showDirtyState false ( + unset -v GIT_PS1_SHOWDIRTYSTATE + __git_ps1 $actual + ) + test_cmp expected $actual +' + +test_expect_success 'prompt - dirty status indicator - shell variable unset with config enabled' ' + printf (master) expected + echo dirty file + test_when_finished git reset --hard + test_config bash.showDirtyState true + ( + unset -v GIT_PS1_SHOWDIRTYSTATE + __git_ps1 $actual + ) + test_cmp expected $actual +' + +test_expect_success 'prompt - dirty status indicator - shell variable set with config disabled' ' + printf (master) expected + echo dirty file + test_when_finished git reset --hard + test_config bash.showDirtyState false + ( + GIT_PS1_SHOWDIRTYSTATE=y + __git_ps1 $actual + ) + test_cmp expected $actual +' + +test_expect_success 'prompt - dirty status indicator - shell variable set with config enabled' ' + printf (master *) expected + echo dirty file + test_when_finished git reset --hard + test_config bash.showDirtyState true + ( GIT_PS1_SHOWDIRTYSTATE=y __git_ps1 $actual ) -- 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: [PATCH v2 3/3] t9903: add extra tests for bash.showDirtyState
On Wed, 2013-02-13 at 08:28 -0800, Junio C Hamano wrote: Martin Erik Werner martinerikwer...@gmail.com writes: Added 3 extra tests for the bash.showDirtyState config option, tests should now cover all combinations of the shell var being set/unset and the config option being enabled/disabled, given a dirty file. Strictly speaking, you have 6 not 4 combinations (shell variable set/unset * config missing/set to false/set to true). I think these additional tests cover should all 6 because config missing case should already have had tests before bash.showDirtyState was added. Indeed, I only mentioned 4 since the other ones existed already, and I didn't change them, but maybe it should be mentioned as combined with previous tests (...) cover all 6 combinations (...) then? * Renamed test 'disabled by config' to 'shell variable set with config disabled' for consistency --- Sign-off? Ah, just forgot the -s flag on that commit, yes it should be Signed-off by me. t/t9903-bash-prompt.sh | 38 +- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index cb008e2..fa81b09 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -360,12 +360,48 @@ test_expect_success 'prompt - dirty status indicator - before root commit' ' test_cmp expected $actual ' -test_expect_success 'prompt - dirty status indicator - disabled by config' ' +test_expect_success 'prompt - dirty status indicator - shell variable unset with config disabled' ' printf (master) expected echo dirty file test_when_finished git reset --hard test_config bash.showDirtyState false ( + unset -v GIT_PS1_SHOWDIRTYSTATE + __git_ps1 $actual + ) + test_cmp expected $actual +' + +test_expect_success 'prompt - dirty status indicator - shell variable unset with config enabled' ' + printf (master) expected + echo dirty file + test_when_finished git reset --hard + test_config bash.showDirtyState true + ( + unset -v GIT_PS1_SHOWDIRTYSTATE + __git_ps1 $actual + ) + test_cmp expected $actual +' + +test_expect_success 'prompt - dirty status indicator - shell variable set with config disabled' ' + printf (master) expected + echo dirty file + test_when_finished git reset --hard + test_config bash.showDirtyState false + ( + GIT_PS1_SHOWDIRTYSTATE=y + __git_ps1 $actual + ) + test_cmp expected $actual +' + +test_expect_success 'prompt - dirty status indicator - shell variable set with config enabled' ' + printf (master *) expected + echo dirty file + test_when_finished git reset --hard + test_config bash.showDirtyState true + ( GIT_PS1_SHOWDIRTYSTATE=y __git_ps1 $actual ) -- 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: [PATCH v2 3/3] t9903: add extra tests for bash.showDirtyState
Martin Erik Werner martinerikwer...@gmail.com writes: Strictly speaking, you have 6 not 4 combinations (shell variable set/unset * config missing/set to false/set to true). I think these additional tests cover should all 6 because config missing case should already have had tests before bash.showDirtyState was added. Indeed, I only mentioned 4 since the other ones existed already, and I didn't change them, but maybe it should be mentioned as combined with previous tests (...) cover all 6 combinations (...) then? It should be sufficient to change the third line of your original to say the config option being missing/enabled/disabled, given a dirty file. and nothing else, I think. Sign-off? Ah, just forgot the -s flag on that commit, yes it should be Signed-off by me. OK, I'll locally amend the patch. Thanks. -- 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: [PATCH v2 3/3] t9903: add extra tests for bash.showDirtyState
On Wed, 2013-02-13 at 11:53 -0800, Junio C Hamano wrote: Martin Erik Werner martinerikwer...@gmail.com writes: Strictly speaking, you have 6 not 4 combinations (shell variable set/unset * config missing/set to false/set to true). I think these additional tests cover should all 6 because config missing case should already have had tests before bash.showDirtyState was added. Indeed, I only mentioned 4 since the other ones existed already, and I didn't change them, but maybe it should be mentioned as combined with previous tests (...) cover all 6 combinations (...) then? It should be sufficient to change the third line of your original to say the config option being missing/enabled/disabled, given a dirty file. and nothing else, I think. Sign-off? Ah, just forgot the -s flag on that commit, yes it should be Signed-off by me. OK, I'll locally amend the patch. Thanks. Ok, so I shouldn't reroll them with s/unset -v/sane_unset/ and reworded commits + sign-off then, I can if you prefer that? -- Martin Erik Werner martinerikwer...@gmail.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: [PATCH v2 3/3] t9903: add extra tests for bash.showDirtyState
Martin Erik Werner martinerikwer...@gmail.com writes: OK, I'll locally amend the patch. Thanks. Ok, so I shouldn't reroll them with s/unset -v/sane_unset/ and reworded commits + sign-off then, I can if you prefer that? You can if you wanted to. That would be less work for me ;-). -- 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
