On Wed, Feb 3, 2021 at 5:51 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > "Lance Ward via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > > t/t7527-status-color-pipe.sh | 55 ++++++++++++++++++++++++++++++++++++ > > Don't we already have test that checks "status" output including its > coloring already? I'd rather see us adding to existing test script, > than allocating a new number for a small subset of features being > tested for a command. After all, test numbers are limited resource. t7508 seems like a good place for these tests. > > +test_expect_success 'git status' ' > > + git status >raw && > > + test_decode_color <raw >out && > > + grep "original$" out > > +' > > Not "new file: *original$" or something less false-positive prone? The "new file:" message is localized, so this `grep` will need to become `test_i18ngrep` again (as it was in the original submission) if this approach is adopted, which is fine. > > +test_expect_success 'git -c color.status=never status' ' > > + git -c color.status=never status >raw && > > + test_decode_color <raw >out && > > + grep "original$" out > > +' > > Would it make sense to have tests for color.status=true, I wonder. > It requires tty to actually "see" the colors output but sending > the output to tty is the normal use case, so we should care... I wondered if `color.status=true` might already be tested by t7508 but apparently it only checks `auto`. > > +test_expect_success 'git -c color.status=always status -v' ' > > + git -c color.status=always status -v >raw && > > + test_decode_color <raw >out && > > + grep "<CYAN>@@ -0,0 +1 @@<RESET>" out && > > + grep "GREEN>+<RESET><GREEN>1<RESET>" out > > +' > > Are we forcing the standard palette? As this is a stand-alone test script with a well-controlled initial configuration, I expect it would be using the default palette. t7508 does assign a custom palette for `color.status` but not, apparently, for `color.diff`, so it presumably should be okay there, as well.
