Re: [PATCH v2 3/3] t9903: add extra tests for bash.showDirtyState
Martin Erik Werner 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
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 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 -- 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 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 08:28 -0800, Junio C Hamano wrote: > Martin Erik Werner 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 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
[PATCH v2 3/3] t9903: add extra tests for bash.showDirtyState
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. * Renamed test 'disabled by config' to 'shell variable set with config disabled' for consistency --- 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" ) && -- 1.7.10.4 -- 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
