All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeremy Kerr <jk@ozlabs.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org, linux-efi@vger.kernel.org,
	Matt Fleming <matt.fleming@intel.com>,
	Lingzhu Xiang <lxiang@redhat.com>, Dave Young <dyoung@redhat.com>
Subject: Re: [PATCH 1/3 v3] selftests: Add tests for efivarfs
Date: Fri, 08 Feb 2013 18:05:52 +0800	[thread overview]
Message-ID: <5114CE00.6050307@ozlabs.org> (raw)
In-Reply-To: <20130207151333.f01d415c.akpm@linux-foundation.org>

Hi Andrew,

Thanks for taking a look at these.

>> @@ -1,4 +1,4 @@
>> -TARGETS = breakpoints kcmp mqueue vm cpu-hotplug memory-hotplug
>> +TARGETS = breakpoints kcmp mqueue vm cpu-hotplug memory-hotplug efivarfs
>
> bah.  This sort of Makefile construct is a wonderful source of patch
> rejects and fixups.  I'll covert this to
>
> --- a/tools/testing/selftests/Makefile~a
> +++ a/tools/testing/selftests/Makefile
> @@ -1,4 +1,11 @@
> -TARGETS = breakpoints epoll kcmp mqueue vm cpu-hotplug memory-hotplug efivarfs
> +TARGETS = breakpoints
> +TARGETS += epoll
> +TARGETS += kcmp
> +TARGETS += mqueue
> +TARGETS += vm
> +TARGETS += cpu-hotplug
> +TARGETS += memory-hotplug
> +TARGETS += efivarfs

Much better, thanks. I'd already had a collision with the epoll tests...

> I'll do this for now:
>
> --- a/tools/testing/selftests/efivarfs/Makefile~selftests-add-tests-for-efivarfs-fix
> +++ a/tools/testing/selftests/efivarfs/Makefile
> @@ -6,7 +6,7 @@ test_objs = open-unlink
>   all: $(test_objs)
>
>   run_tests: all
> -	@./efivarfs.sh || echo "efivarfs selftests: [FAIL]"
> +	@/bin/sh ./efivarfs.sh || echo "efivarfs selftests: [FAIL]"
>
>   clean:
>   	rm -f $(test_objs)
>
> but I'm not sure I did it right :(

efivarfs.sh requires bash currently, so we'll need to call this explicitly:

+	@/bin/bash ./efivarfs.sh || echo "efivarfs selftests: [FAIL]"

Is this okay?

> The general ruleset for selftests is: do as much as you can if you're not
> root and don't take too long and don't break the build on any
> architecture and don't cause the top-level "make run_tests" to fail if
> your feature is unconfigured.

Ah, good stuff to know. I'll send a patch adding this info to 
Documentation/ too.

> Does this code pass all that?

It should, yes:

  * all test requires root at present, as all efivarfs files are only
    writable by root

  * the built binaries doesn't use anything more than basic C, so should
    build fine wherever we have gcc.

  * efivarfs.sh will skip all tests if efivarfs is not mounted

However, the tests expose a bug at the moment, so run_tests will fail. 
Matt will have that fixed soon though :)

Cheers,


Jeremy



WARNING: multiple messages have this Message-ID (diff)
From: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
To: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Matt Fleming
	<matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	Lingzhu Xiang <lxiang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Subject: Re: [PATCH 1/3 v3] selftests: Add tests for efivarfs
Date: Fri, 08 Feb 2013 18:05:52 +0800	[thread overview]
Message-ID: <5114CE00.6050307@ozlabs.org> (raw)
In-Reply-To: <20130207151333.f01d415c.akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>

Hi Andrew,

Thanks for taking a look at these.

>> @@ -1,4 +1,4 @@
>> -TARGETS = breakpoints kcmp mqueue vm cpu-hotplug memory-hotplug
>> +TARGETS = breakpoints kcmp mqueue vm cpu-hotplug memory-hotplug efivarfs
>
> bah.  This sort of Makefile construct is a wonderful source of patch
> rejects and fixups.  I'll covert this to
>
> --- a/tools/testing/selftests/Makefile~a
> +++ a/tools/testing/selftests/Makefile
> @@ -1,4 +1,11 @@
> -TARGETS = breakpoints epoll kcmp mqueue vm cpu-hotplug memory-hotplug efivarfs
> +TARGETS = breakpoints
> +TARGETS += epoll
> +TARGETS += kcmp
> +TARGETS += mqueue
> +TARGETS += vm
> +TARGETS += cpu-hotplug
> +TARGETS += memory-hotplug
> +TARGETS += efivarfs

Much better, thanks. I'd already had a collision with the epoll tests...

> I'll do this for now:
>
> --- a/tools/testing/selftests/efivarfs/Makefile~selftests-add-tests-for-efivarfs-fix
> +++ a/tools/testing/selftests/efivarfs/Makefile
> @@ -6,7 +6,7 @@ test_objs = open-unlink
>   all: $(test_objs)
>
>   run_tests: all
> -	@./efivarfs.sh || echo "efivarfs selftests: [FAIL]"
> +	@/bin/sh ./efivarfs.sh || echo "efivarfs selftests: [FAIL]"
>
>   clean:
>   	rm -f $(test_objs)
>
> but I'm not sure I did it right :(

efivarfs.sh requires bash currently, so we'll need to call this explicitly:

+	@/bin/bash ./efivarfs.sh || echo "efivarfs selftests: [FAIL]"

Is this okay?

> The general ruleset for selftests is: do as much as you can if you're not
> root and don't take too long and don't break the build on any
> architecture and don't cause the top-level "make run_tests" to fail if
> your feature is unconfigured.

Ah, good stuff to know. I'll send a patch adding this info to 
Documentation/ too.

> Does this code pass all that?

It should, yes:

  * all test requires root at present, as all efivarfs files are only
    writable by root

  * the built binaries doesn't use anything more than basic C, so should
    build fine wherever we have gcc.

  * efivarfs.sh will skip all tests if efivarfs is not mounted

However, the tests expose a bug at the moment, so run_tests will fail. 
Matt will have that fixed soon though :)

Cheers,


Jeremy

  parent reply	other threads:[~2013-02-08 10:06 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-06 14:48 [PATCH 0/3 v3] selftests: Add efivarfs tests Jeremy Kerr
2013-02-06 14:48 ` [PATCH 2/3 v3] selftests/efivarfs: Add empty file creation test Jeremy Kerr
2013-02-06 14:48 ` [PATCH 1/3 v3] selftests: Add tests for efivarfs Jeremy Kerr
2013-02-07 23:13   ` Andrew Morton
2013-02-07 23:13     ` Andrew Morton
2013-02-08 10:02     ` [PATCH] Documentation: Add a simple doc for selftests Jeremy Kerr
2013-02-12 23:56       ` Andrew Morton
2013-02-08 10:05     ` Jeremy Kerr [this message]
2013-02-08 10:05       ` [PATCH 1/3 v3] selftests: Add tests for efivarfs Jeremy Kerr
2013-02-08 10:08       ` Matt Fleming
2013-02-08 10:08         ` Matt Fleming
2013-02-12 23:50         ` Andrew Morton
2013-02-12 23:50           ` Andrew Morton
2013-02-13  7:32           ` Matt Fleming
2013-02-13  7:32             ` Matt Fleming
2013-02-12 23:48       ` Andrew Morton
2013-02-12 23:48         ` Andrew Morton
2013-02-06 14:48 ` [PATCH 3/3 v3] selftests/efivarfs: Add create-read test Jeremy Kerr

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=5114CE00.6050307@ozlabs.org \
    --to=jk@ozlabs.org \
    --cc=akpm@linux-foundation.org \
    --cc=dyoung@redhat.com \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lxiang@redhat.com \
    --cc=matt.fleming@intel.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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.