git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Stefan Beller <sbeller@google.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	Git Mailing List <git@vger.kernel.org>, Joey Hess <id@joeyh.name>
Subject: Re: [PATCH] tests: add an optional test to test git-annex
Date: Wed, 17 May 2017 16:03:48 -0700	[thread overview]
Message-ID: <CAGZ79kYX8ct4GKDbZxGJmR5kkVrs4zjnaKOaD8Dm8rKx8aPA+Q@mail.gmail.com> (raw)
In-Reply-To: <CACBZZX5BrASFemN2VviMjH-AqnxU6veLVmjRdn1iYuA9fgKQog@mail.gmail.com>

On Wed, May 17, 2017 at 12:33 PM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> On Wed, May 17, 2017 at 12:19 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>>
>>>> Well, it is one thing to place git-annex under CI to make sure its
>>>> latest and greatest works together well with our latest and greatest
>>>> (and it may be something we want to see happen), but driving its
>>>> tests from our testsuite sounds like a tail wagging the dog, at
>>>> least to me.
>>>
>>> To me this is just a question of:
>>>
>>> * Is it the case that git-annex tests for a lot of edge cases we don't
>>> test for: Yes, probably. As evidenced by them spotting this
>>> regression, and not us.
>>
>> And I'd encourage them to keep doing so.
>
> The point of this patch is that we can do this more systematically and
> reliably, not have people discover this sort of thing after a major
> release.
>
> I.e. we can be pro-active about this instead of waiting for bug
> reports to roll in.

We can be pro-active without this patch, too. ;)
I guess this makes it just easier for someone out of the Git community to
be proactive in searching for defects.

>
> The utility of this test is that sometime close to release someone
> (e.g. me) can run it, if it fails let's see if it fails on the last
> release version of ours, if so it's probably upstream breakage, or
> like with the 2.13.0 release if it's OK on 2.12.0 it's our bug.
>
> It'll never trip some random tester up since you need to explicitly
> opt-in via EXTERNAL_TESTS=1, so honestly I'm a bit puzzled by these
> objections. This incurs no burden on either devs, packagers or users,
> and would have demonstrably detected an issue we'd rather have wanted
> to know about pre-release than post-release as is the case now.

I think this patch could spark a discussion what we expect from our test suite.
Is it a contract that we promise, as in "Here are some blessed workflows,
our users should see and imitate"?

This patch sort of feels like (a) we let others dictate the contract to us
and (b) we choose the easy way out as we do not write enough tests on
our own, so we force downstream to help us out.

I guess it is appropriate to compare this level of testing to other external
tests, such as cppcheck, travis, coverity. For some of them (travis) we
carry some code with us (.travis), not so for others (coverity would benefit
from [1] that I carry outside of Junios tree). Admittedly these external tools
are all focused on testing, not on building on top of Git, which brings
in other expectations from these tools.

[1] https://github.com/stefanbeller/git/commit/0781fbafb9dd2a995ba62a9af5f7581e3cf05359

---
As mentioned in the other thread[2], the idea was floated to have this test or
the downstream project as a submodule, and if you are interested in the test,
then you would obtain the submodule and run the tests (in there?)
That would have the benefit of less 'uninteresting' glue logic for those who
are not interested in these tests as gitlinks may be easier to ignore
for a developer
than code. (Less data to fetch, git-grep also doesn't yield results
for the uninteresting
parts as these submodules would be uninitialized).
(The more I talk about this "downstream tests in submodules"
idea, the less convinced I am)

[2] https://public-inbox.org/git/20170517113824.31700-1-avarab@gmail.com/

  reply	other threads:[~2017-05-17 23:03 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-16 17:10 reversion in GIT_COMMON_DIR refs path Joey Hess
2017-05-16 17:50 ` Ævar Arnfjörð Bjarmason
2017-05-16 17:59   ` Joey Hess
2017-05-16 20:37     ` [PATCH] tests: add an optional test to test git-annex Ævar Arnfjörð Bjarmason
2017-05-16 22:10       ` Joey Hess
2017-05-17  2:45       ` Junio C Hamano
2017-05-17  6:47         ` Ævar Arnfjörð Bjarmason
2017-05-17 10:19           ` Junio C Hamano
2017-05-17 19:33             ` Ævar Arnfjörð Bjarmason
2017-05-17 23:03               ` Stefan Beller [this message]
2017-05-17 23:59                 ` Brandon Williams
2017-05-17 23:56             ` Brandon Williams
2017-05-19 14:37 ` reversion in GIT_COMMON_DIR refs path Joey Hess
2017-05-22 11:11   ` Duy Nguyen

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=CAGZ79kYX8ct4GKDbZxGJmR5kkVrs4zjnaKOaD8Dm8rKx8aPA+Q@mail.gmail.com \
    --to=sbeller@google.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=id@joeyh.name \
    /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).