[systemd-devel] [PATCH v3] Skip tests that depend on /etc/machine-id if it is not present

David Strauss david at davidstrauss.net
Fri Nov 30 16:59:14 PST 2012


A few things:

   - Please fix the spelling of "present."
   - The number -128, if used, should get #define'd instead of being a
   magic number dropped in a bunch of places.
   - It's not good to mask all errors opening the machine-id with the -128
   response code. The change here seems to make all opening errors return -128.

More importantly, what's wrong with looking for the normal [Errno 2] "No
such file or directory" return code?


On Fri, Nov 30, 2012 at 6:52 AM, Ramkumar Ramachandra <artagnon at gmail.com>wrote:

> The following tests fail if /etc/machine-id is not present:
>
>   $ ./test-id128
>   random: a08ea8ed34594d4bbd953dd182ec86f9
>   Assertion 'sd_id128_get_machine(&id) == 0' failed at
>   src/test/test-id128.c:41, function main(). Aborting.
>   [1]    8017 abort (core dumped)  ./test-id128
>
>   $ ./test-journal
>   Assertion 'journal_file_open("test.journal", O_RDWR|O_CREAT, 0666,
>   true, true, NULL, NULL, NULL, &f) == 0' failed at
>   src/journal/test-journal.c:46, function main(). Aborting.
>   [1]    8059 abort (core dumped)  ./test-journal
>
>   $ ./test-journal-stream
>   Assertion 'journal_file_open("one.journal", O_RDWR|O_CREAT, 0666,
>   true, false, NULL, NULL, NULL, &one) == 0' failed at
>   src/journal/test-journal-stream.c:88, function main(). Aborting.
>   [1]    8107 abort (core dumped)  ./test-journal-stream
>
>   $ ./test-journal-verify
>   Generating...
>   Assertion 'journal_file_open("test.journal", O_RDWR|O_CREAT, 0666,
>   true, !!verification_key, NULL, NULL, NULL, &f) == 0' failed at
>   src/journal/test-journal-verify.c:87, function main(). Aborting.
>   [1]    8154 abort (core dumped)  ./test-journal-verify
>
> This is because they call sd_id128_get_machine() which barfs if
> /etc/machine-id can't be open()'ed.  Treat this as a special case and
> skip the dependent tests instead of failing them.
> ---
>  Oops; forgot to declate `id' in the previous iteration.
>
>  src/journal/test-journal-stream.c |    6 ++++++
>  src/journal/test-journal-verify.c |    6 ++++++
>  src/journal/test-journal.c        |    6 ++++++
>  src/libsystemd-id128/sd-id128.c   |    2 +-
>  src/test/test-id128.c             |    8 ++++++--
>  5 files changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/src/journal/test-journal-stream.c
> b/src/journal/test-journal-stream.c
> index b3e816d..d07448f 100644
> --- a/src/journal/test-journal-stream.c
> +++ b/src/journal/test-journal-stream.c
> @@ -79,6 +79,12 @@ int main(int argc, char *argv[]) {
>          char *z;
>          const void *data;
>          size_t l;
> +        sd_id128_t id;
> +
> +        if (sd_id128_get_machine(&id) == -128) {
> +                printf("skipping test: /etc/machine-id not preset\n");
> +                return 0;
> +        }
>
>          log_set_max_level(LOG_DEBUG);
>
> diff --git a/src/journal/test-journal-verify.c
> b/src/journal/test-journal-verify.c
> index b667721..ee55485 100644
> --- a/src/journal/test-journal-verify.c
> +++ b/src/journal/test-journal-verify.c
> @@ -76,6 +76,12 @@ int main(int argc, char *argv[]) {
>          char c[FORMAT_TIMESPAN_MAX];
>          struct stat st;
>          uint64_t p;
> +        sd_id128_t id;
> +
> +        if (sd_id128_get_machine(&id) == -128) {
> +                printf("skipping test: /etc/machine-id not preset\n");
> +                return 0;
> +        }
>
>          log_set_max_level(LOG_DEBUG);
>
> diff --git a/src/journal/test-journal.c b/src/journal/test-journal.c
> index f4dc52c..192b787 100644
> --- a/src/journal/test-journal.c
> +++ b/src/journal/test-journal.c
> @@ -37,6 +37,12 @@ int main(int argc, char *argv[]) {
>          Object *o;
>          uint64_t p;
>          char t[] = "/tmp/journal-XXXXXX";
> +        sd_id128_t id;
> +
> +        if (sd_id128_get_machine(&id) == -128) {
> +                printf("skipping test: /etc/machine-id not preset\n");
> +                return 0;
> +        }
>
>          log_set_max_level(LOG_DEBUG);
>
> diff --git a/src/libsystemd-id128/sd-id128.c
> b/src/libsystemd-id128/sd-id128.c
> index 4286ae7..4e5aaad 100644
> --- a/src/libsystemd-id128/sd-id128.c
> +++ b/src/libsystemd-id128/sd-id128.c
> @@ -106,7 +106,7 @@ _public_ int sd_id128_get_machine(sd_id128_t *ret) {
>
>          fd = open("/etc/machine-id", O_RDONLY|O_CLOEXEC|O_NOCTTY);
>          if (fd < 0)
> -                return -errno;
> +                return -128; /* Special return value */
>
>          k = loop_read(fd, buf, 32, false);
>          close_nointr_nofail(fd);
> diff --git a/src/test/test-id128.c b/src/test/test-id128.c
> index bfd743e..ea621f9 100644
> --- a/src/test/test-id128.c
> +++ b/src/test/test-id128.c
> @@ -38,8 +38,12 @@ int main(int argc, char *argv[]) {
>          assert_se(sd_id128_from_string(t, &id2) == 0);
>          assert_se(sd_id128_equal(id, id2));
>
> -        assert_se(sd_id128_get_machine(&id) == 0);
> -        printf("machine: %s\n", sd_id128_to_string(id, t));
> +        if (sd_id128_get_machine(&id) == -128)
> +                printf("skipping test: /etc/machine-id not preset\n");
> +        else {
> +                assert_se(sd_id128_get_machine(&id) == 0);
> +                printf("machine: %s\n", sd_id128_to_string(id, t));
> +        }
>
>          assert_se(sd_id128_get_boot(&id) == 0);
>          printf("boot: %s\n", sd_id128_to_string(id, t));
> --
> 1.7.8.1.362.g5d6df.dirty
>
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>



-- 

David Strauss
   | david at davidstrauss.net
   | +1 512 577 5827 [mobile]
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20121130/d9e8bba3/attachment-0001.html>


More information about the systemd-devel mailing list