git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Elijah Newren <newren@gmail.com>
To: Jeff King <peff@peff.net>
Cc: Stefan Beller <sbeller@google.com>,
	Junio C Hamano <gitster@pobox.com>,
	Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH 0/6] Add merge recursive testcases with undetected conflicts
Date: Tue, 10 Jul 2018 08:42:04 -0700	[thread overview]
Message-ID: <CABPp-BG5Rn=3MBS+daSJ+2rLsZWcswP3=8zX-F=5ncdzc9y=SQ@mail.gmail.com> (raw)
In-Reply-To: <20180710044456.GA1870@sigill.intra.peff.net>

On Mon, Jul 9, 2018 at 9:44 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Jul 09, 2018 at 01:22:29PM -0700, Elijah Newren wrote:
>
>> Oh, I didn't know about test-lint.  Is there a place that documents
>> the various checks you run, so I can avoid slowing you down?  Ones I
>> know about:
>>
>> Already documented:
>>   * `make DEVELOPER=1` (from CodingGuidelines)
>>   * running tests (from SubmittingPatches)
>>
>> Stuff I've seen you mention in emails over time:
>>   * linux/scripts/checkpatch.pl
>>   * git grep -e '\<inline\>' --and --not -e 'static inline' -- \*.h
>>   * make -C t/ test-lint
>
> test-lint is supposed to be run automatically as part of "make test" (or
> "make prove"), unless you've specifically disabled it by setting
> TEST_LINT. And it does complain for me with your patches. If it doesn't
> for you, then we have a bug to fix. :)

Oh, this may be my bad.  Years ago someone pointed out that the
testsuite could be run under 'prove', which provided nicer output and
made sure to run the longest tests (e.g. the horrifically slow
t9001-send-email.sh) first.  So my test alias is:

   time prove -j7 --timer --state failed,slow,save t[0-9]*.sh ::
"--root=/dev/shm"

(with possibly different -j settings on different machines) and I just
stopped running make test.  Didn't learn about the 'make prove'
target, even though it's apparently now been there for nearly 8 years.

> I won't be surprised, though, if you just ran "./t6036" manually before
> sending, since your patches literally didn't touch any other files.

That may also be true, though I would have missed it even if I had
code changes due to not being aware of 'make prove'.

> In theory we could push some of the linting down into the test scripts
> themselves (some of it, like the &&-linter, is there already by
> necessity). But it might also end up annoying, since usually dropping
> down to manual single-test runs means you're trying to debug something,
> and extra linting processes could get in the way.
>
>> Are there others?
>
> I like:
>
>   make SANITIZE=address,undefined test
>
> though it's pretty heavy-weight (but not nearly as much as valgrind).
> You probably also need BLK_SHA1=Yes, since the default DC_SHA1 has some
> unaligned loads that make UBSan complain. We should maybe teach the
> Makefile to do that by default.
>
> I've also been playing with clang's scan-build. It _did_ find a real bug
> recently, but it has a bunch of false positives.
>
> Stefan runs Coverity against pu periodically. IIRC It's a pain to run
> yourself, but the shared results can be mailed to you, or you can poke
> around at https://scan.coverity.com/projects/git. That _also_ has a ton
> of false positives, but it's good about cataloguing them so the periodic
> email usually just mentions the new ones.

Cool, thanks for the pointers.

  reply	other threads:[~2018-07-10 15:42 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-01  4:11 [PATCH 0/6] Add merge recursive testcases with undetected conflicts Elijah Newren
2018-07-01  4:11 ` [PATCH 1/6] t6036: add a failed conflict detection case with symlink modify/modify Elijah Newren
2018-07-01  4:11 ` [PATCH 2/6] t6036: add a failed conflict detection case with symlink add/add Elijah Newren
2018-07-01  4:11 ` [PATCH 3/6] t6036: add a failed conflict detection case with submodule modify/modify Elijah Newren
2018-07-01  4:11 ` [PATCH 4/6] t6036: add a failed conflict detection case with submodule add/add Elijah Newren
2018-07-01  4:11 ` [PATCH 5/6] t6036: add a failed conflict detection case with conflicting types Elijah Newren
2018-07-01  4:11 ` [PATCH 6/6] t6036: add a failed conflict detection case: regular files, different modes Elijah Newren
2018-07-09 17:53 ` [PATCH 0/6] Add merge recursive testcases with undetected conflicts Junio C Hamano
2018-07-09 20:22   ` Elijah Newren
2018-07-10  4:44     ` Jeff King
2018-07-10 15:42       ` Elijah Newren [this message]
2018-07-10 17:19         ` Jeff King
2018-07-11  4:02     ` Elijah Newren
2018-07-11 15:40       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CABPp-BG5Rn=3MBS+daSJ+2rLsZWcswP3=8zX-F=5ncdzc9y=SQ@mail.gmail.com' \
    --to=newren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).