FSTests Archive on lore.kernel.org
 help / color / Atom feed
From: Eric Sandeen <sandeen@sandeen.net>
To: Pavel Reichl <preichl@redhat.com>, Eryu Guan <eguan@linux.alibaba.com>
Cc: fstests@vger.kernel.org, zlang@redhat.com, guan@eryu.me,
	Eric Sandeen <sandeen@redhat.com>,
	Dave Chinner <dchinner@redhat.com>
Subject: Re: [PATCH v2 1/4] common/rc: Add _require_{chown,chmod,symlink}()
Date: Wed, 7 Apr 2021 11:47:04 -0500
Message-ID: <38e0b763-3d0b-9fdb-7df0-b3c526785364@sandeen.net> (raw)
In-Reply-To: <9f6d45ca-7f96-1cfa-5e66-a355ad4e0c7a@redhat.com>

On 4/1/21 4:47 AM, Pavel Reichl wrote:
>> We only use whitelist if it's impossible to do such check.
>>
>> Thanks,
>> Eryu
>>
> Hi, 
> 
> it does fail. It was actually my original intention to write the _require*() so it would check if the command succeeds as you are suggesting.
> 
> However, Eric and Dave were worried that adding more _require*() through the tests would lead to further slowing test execution. This worry actually makes sense to me.
> 
> Is there a significant benefit of testing the support vs. maintaining check based on FSTYP variable? I guess doing the check saves us from the need to update the code when new file-system is added, however actually doing the check increases time of test execution (but I haven't done any measurements yet - it's just my guess).
> 
> I really don't mind doing it either way and I'm happy to change the code - I'm just trying to explain :-)

that's my fault, sorry.  Dave had expressed some concern about exfat changes slowing down testing for every other fs, and we talked about a whitelist.  But now that I think about it, I'm not sure this functiona test takes any real extra time.

Pavel, maybe you can just evaluate whether there really is any significant time difference, and if there is not, go back to the functional test.

Sorry for the hassle.

-Eric

  reply index

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-30 22:00 [PATCH v2 0/4] Fix some tests that fail for exfat FS Pavel Reichl
2021-03-30 22:00 ` [PATCH v2 1/4] common/rc: Add _require_{chown,chmod,symlink}() Pavel Reichl
2021-03-31  0:25   ` Zorro Lang
2021-03-31  7:17     ` Pavel Reichl
2021-04-01  3:38   ` Eryu Guan
2021-04-01  9:47     ` Pavel Reichl
2021-04-07 16:47       ` Eric Sandeen [this message]
2021-03-30 22:00 ` [PATCH v2 2/4] common: hide permision warning from mkswap for exfat Pavel Reichl
2021-04-07 17:03   ` Eric Sandeen
2021-04-15  9:15     ` Pavel Reichl
2021-03-30 22:00 ` [PATCH v2 3/4] generic/554: hide permision warning on exfat Pavel Reichl
2021-04-01  3:40   ` Eryu Guan
2021-03-30 22:00 ` [PATCH v2 4/4] generic/003: Amend the test for exfat Pavel Reichl
2021-04-07 18:03   ` Eric Sandeen
2021-04-15  9:14     ` Pavel Reichl

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

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

  git send-email \
    --in-reply-to=38e0b763-3d0b-9fdb-7df0-b3c526785364@sandeen.net \
    --to=sandeen@sandeen.net \
    --cc=dchinner@redhat.com \
    --cc=eguan@linux.alibaba.com \
    --cc=fstests@vger.kernel.org \
    --cc=guan@eryu.me \
    --cc=preichl@redhat.com \
    --cc=sandeen@redhat.com \
    --cc=zlang@redhat.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

FSTests Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/fstests/0 fstests/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 fstests fstests/ https://lore.kernel.org/fstests \
		fstests@vger.kernel.org
	public-inbox-index fstests

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.fstests


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git