From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-3.9 required=3.0 tests=AWL,BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by dcvr.yhbt.net (Postfix) with ESMTP id CC2AA1F453 for ; Wed, 30 Jan 2019 11:47:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727750AbfA3Lry (ORCPT ); Wed, 30 Jan 2019 06:47:54 -0500 Received: from mail-qk1-f195.google.com ([209.85.222.195]:34139 "EHLO mail-qk1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727009AbfA3Lry (ORCPT ); Wed, 30 Jan 2019 06:47:54 -0500 Received: by mail-qk1-f195.google.com with SMTP id q8so13502599qke.1 for ; Wed, 30 Jan 2019 03:47:52 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=AlXiLZb2PUH8afQWrtKTE9dJA2CncIxK5tnl8/g/RXA=; b=b465Klia+2jZfTz4hozDNhV5eQq+CDZOeOZAVd0Xo8erBZk6SG2QGlRt8hSNIS/Fr7 L33AMoFsPxZzUkHc2/bnXBy4fcEQ09P6Xr4Yx7UY4wUopTpdW2uUG8yT0GSfQRs7m9zr K42YeD1qqA+r+GFHz9CxmNMXYtBJNoYLkv5w7r5P4cOoy+r9/c3c3YDhb0qccbnyKCMu jnha647zkXH5lYsb+cctM64F/OU1M1tl4sayo05U/6clw2dzSEPfFUBO3RSVxyKbQW5S ica4rpFLME9jcAdu6jcO0xtXS2xPtudficCqMri6LCLUVclaAX1pJvMxiZ2s10WxDY8B IJWQ== X-Gm-Message-State: AJcUukdPH5bGZydOsueFRF+cqEhhG3Qfghxqmi0AdQLW3ovKdQ192Hd4 wC+Dqj9CimHjvIm39M3AATjNlSKw1e9TKtGaTdE= X-Google-Smtp-Source: ALg8bN6N94E8cbf0QmiGOvjCBqRZG3t7RoaW1wor0erLBjB9hhMgHqdAB+uyr90EbfU1hH/XdgfF8+RVFs7Wupe490w= X-Received: by 2002:a37:5e42:: with SMTP id s63mr26306478qkb.220.1548848872372; Wed, 30 Jan 2019 03:47:52 -0800 (PST) MIME-Version: 1.0 References: <20190129193818.8645-1-jeremyhu@apple.com> <20190129193818.8645-4-jeremyhu@apple.com> In-Reply-To: <20190129193818.8645-4-jeremyhu@apple.com> From: Eric Sunshine Date: Wed, 30 Jan 2019 06:47:41 -0500 Message-ID: Subject: Re: [PATCH (Apple Git) 03/13] t0500: New regression test for git add of a path that contains a .git directory To: Jeremy Huddleston Sequoia Cc: Git List , Jeff King Content-Type: text/plain; charset="UTF-8" Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On Tue, Jan 29, 2019 at 4:19 PM Jeremy Huddleston Sequoia wrote: > Subject: t0500: New regression test for git add of a path that contains a .git directory Please describe the actual problem here in the commit message so readers of this change can understand what this is all about. > Signed-off-by: Jeremy Huddleston Sequoia > --- > diff --git a/t/t0500-apple.sh b/t/t0500-apple.sh > @@ -0,0 +1,40 @@ > +#!/bin/sh > +# > +# Copyright (c) 2012-2016 Apple Inc. > +# > +# Tests for regressions found by Apple Inc. for issues that upstream does not > +# want to fix or accept tests for. This is an odd comment for a patch which is intended to be upstreamed. > +test_description='Apple Inc. specific tests' Is this script actually specific to Apple? If not, a better description is likely warranted. Alternatively, place this new test in an appropriate existing test script. > +# Inaccessible private bug report. Please describe the actual regression here. > +# This test case addresses a regression introduced between v1.7.3 and v1.7.5 > +# git bisect good v1.7.3 > +# git bisect bad v1.7.5 > +# ... > +# found 18e051a3981f38db08521bb61ccf7e4571335353 This commentary isn't very useful going forward, thus not worth having in the script itself, although it may make useful information for the commit message (though more likely not). Usually, such commentary would be placed below the "---" line just under your sign-off. > +test_expect_success ' -- git add of a path that contains a .git directory' ' As above, a better title would be welcome, one which actually means something to people without access to the private bug report. > + rm -rf .git && > + mkdir -p orig/sub/dir/otherdir && > + cd orig/sub && We don't 'cd' around inside tests without ensuring that the 'cd' is undone automatically even if the test fails. (See below.) > + echo "1" > dir/file && > + echo "2" > dir/otherdir/file && > + git init --quiet && Why --quiet? Output generated by commands is already suppressed by default when the test is run normally, but it is useful to have when something goes wrong, so we don't usually want to suppress it manually. Same comment applies to >/dev/null redirects. > + git add -A && > + git commit -m "Initial Commit" --quiet && > + cd - > /dev/null && If something fails above this point, then this "cd -" will never execute, so any tests which get added below this one in the script will operate in the wrong directory. The normal way to 'cd' within a test is within a subshell so the 'cd' is undone automatically whether the test fails or not: ( cd orig/sub ... ) > + git init --bare --quiet "${TESTROOT}/git_dir.git" && > + git --git-dir="${TESTROOT}/git_dir.git" --work-tree=/ add -f -- "${TESTROOT}/orig/sub/" && > + git --git-dir="${TESTROOT}/git_dir.git" --work-tree=/ add -f -- "${TESTROOT}/orig/" && > + git --git-dir="${TESTROOT}/git_dir.git" --work-tree=/ commit -m "Commit." | > + grep -q "2 files changed, 2 insertions" > +' We don't normally place a Git command upstream of a pipe since its exit status will get swallowed by the pipe, thus potentially losing important information. Instead, redirect the command output to a file and 'grep' on the file. Also, the string you're grepping is likely to be localized, so use test_i18ngrep() instead.