| Message ID | 20230417191044.909094-1-rybak.a.v@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | t: fix unused files, part 2 | expand |
Andrei Rybak <rybak.a.v@gmail.com> writes: > Creation of files from redirecting output of Git commands in tests has been > removed for files which aren't being used for assertions. CC'ed are authors of > the affected tests. > > v1 cover letter: > https://lore.kernel.org/git/20230401212858.266508-1-rybak.a.v@gmail.com/ > v2 cover letter: > https://lore.kernel.org/git/20230403223338.468025-1-rybak.a.v@gmail.com/ This round has not seen any further comments; shall we consider it pretty much done and ready to move to 'next' by now? Thanks.
On 01/05/2023 23:52, Junio C Hamano wrote: > Andrei Rybak <rybak.a.v@gmail.com> writes: > >> Creation of files from redirecting output of Git commands in tests has been >> removed for files which aren't being used for assertions. CC'ed are authors of >> the affected tests. >> >> v1 cover letter: >> https://lore.kernel.org/git/20230401212858.266508-1-rybak.a.v@gmail.com/ >> v2 cover letter: >> https://lore.kernel.org/git/20230403223338.468025-1-rybak.a.v@gmail.com/ > > This round has not seen any further comments; shall we consider it > pretty much done and ready to move to 'next' by now? In general, I'm OK with the series as is. While answering Ævar's questions to some of the patches in v2, I went quite deep in trying to investigate what is and isn't important to validate/assert in particular tests, but I haven't come up with a good way to include this information in commit messages for this series. Notes per patch: - 1/6 for t0300 is just an explanation about why one out of three cases in one test does not check stdout (and doesn't need to). https://lore.kernel.org/git/db2de983-9b1f-5efb-0fdc-cc704e6b875b@gmail.com/ - 3/6 for t1300 lead to a separate series https://lore.kernel.org/git/20230423134649.431783-1-rybak.a.v@gmail.com/ - 4/6 for t1450 had an idea for a test 'fresh repository has no dangling objects'. I'm doubtful about usefulness of such a test, so hasn't sent it as a patch yet. https://lore.kernel.org/git/35bc2dc5-d5cb-3492-ff94-41b93b7563d4@gmail.com/ - 6/6 for t2019 -- a dive into how output of "git checkout" is tested https://lore.kernel.org/git/4ef5464b-31dd-3c3e-05be-9891162e4f05@gmail.com/#t Patches 2/6 and 5/6 are different from others, because they fix more obvious issues. > Thanks.
On Mon, May 1, 2023 at 3:04 PM Junio C Hamano <gitster@pobox.com> wrote: > > Andrei Rybak <rybak.a.v@gmail.com> writes: > > > Creation of files from redirecting output of Git commands in tests has been > > removed for files which aren't being used for assertions. CC'ed are authors of > > the affected tests. > > > > v1 cover letter: > > https://lore.kernel.org/git/20230401212858.266508-1-rybak.a.v@gmail.com/ > > v2 cover letter: > > https://lore.kernel.org/git/20230403223338.468025-1-rybak.a.v@gmail.com/ > > This round has not seen any further comments; shall we consider it > pretty much done and ready to move to 'next' by now? I think so. I read through the series. I also read Ævar's and Andrei's extended comments on v2. Ævar does bring up good points about whether we should be testing more, but Andrei I think did a good investigation, cc'ed original code authors (who would be the right ones to comment on whether those other things should be tested), etc. The tests as-is before this series are harder than necessary to understand, and Andrei cleans them up. It feels like good forward progress to me, even if there _might_ be a better eventual optimal. Reviewed-by: Elijah Newren <newren@gmail.com>
Elijah Newren <newren@gmail.com> writes: > On Mon, May 1, 2023 at 3:04 PM Junio C Hamano <gitster@pobox.com> wrote: >> >> Andrei Rybak <rybak.a.v@gmail.com> writes: >> >> > Creation of files from redirecting output of Git commands in tests has been >> > removed for files which aren't being used for assertions. CC'ed are authors of >> > the affected tests. >> > >> > v1 cover letter: >> > https://lore.kernel.org/git/20230401212858.266508-1-rybak.a.v@gmail.com/ >> > v2 cover letter: >> > https://lore.kernel.org/git/20230403223338.468025-1-rybak.a.v@gmail.com/ >> >> This round has not seen any further comments; shall we consider it >> pretty much done and ready to move to 'next' by now? > > I think so. ... > > Reviewed-by: Elijah Newren <newren@gmail.com> Thanks.
Creation of files from redirecting output of Git commands in tests has been removed for files which aren't being used for assertions. CC'ed are authors of the affected tests. v1 cover letter: https://lore.kernel.org/git/20230401212858.266508-1-rybak.a.v@gmail.com/ v2 cover letter: https://lore.kernel.org/git/20230403223338.468025-1-rybak.a.v@gmail.com/ Changes since v2: - Added "Acked-by" of Øystein Walle to patch 5/6 Cf. https://lore.kernel.org/git/CAFaJEqug4bghEMnEQzGDN10EqM8e8iSf5i12AvOm+NZzDCQKOw@mail.gmail.com/ Range diff: 1: 828bb18bd7 = 1: 828bb18bd7 t0300: don't create unused file 2: a5b299a0c6 = 2: a5b299a0c6 t1300: fix config file syntax error descriptions 3: 806df16415 = 3: 806df16415 t1300: don't create unused files 4: 6742c957e5 = 4: 6742c957e5 t1450: don't create unused files 5: 6c173a5c46 ! 5: 19ac488922 t1502: don't create unused files @@ Commit message Don't redirect standard output of "git rev-parse" to file "out" in t1502-rev-parse-parseopt.sh to avoid creating unnecessary files. + Acked-by: Øystein Walle <oystwa@gmail.com> Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com> ## t/t1502-rev-parse-parseopt.sh ## 6: d508c1def3 = 6: c41657be88 t2019: don't create unused files Andrei Rybak (6): t0300: don't create unused file t1300: fix config file syntax error descriptions t1300: don't create unused files t1450: don't create unused files t1502: don't create unused files t2019: don't create unused files t/t0300-credentials.sh | 2 +- t/t1300-config.sh | 10 +++++----- t/t1450-fsck.sh | 5 +---- t/t1502-rev-parse-parseopt.sh | 6 +++--- t/t2019-checkout-ambiguous-ref.sh | 4 ++-- 5 files changed, 12 insertions(+), 15 deletions(-)