[systemd-devel] Fwd: [PATCH v3] tests: added tests for unit_file_get_{state, list}

[Lennart Poettering]

On Fri, 17.10.14 14:02, Martin Pitt (martin.pitt at ubuntu.com) wrote:

> > diff --git a/Makefile.am b/Makefile.am
> > index e52db17..7d4f2f5 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -1358,7 +1358,8 @@ tests += \
> >         test-ratelimit \
> >         test-condition-util \
> >         test-uid-range \
> > -       test-bus-policy
> > +       test-bus-policy \
> > +       test-enabled
> 
> Nitpick: Can we end lists with a $(NULL) line so that patches don't
> always have to change the previous last line?

We have not done this so far in systemd, but sounds like a good scheme
to start adopting here.

> > +static void test_enabled(int argc, char* argv[]) {
> > [...]
> > +        h = hashmap_new(&string_hash_ops);
> > +        r = unit_file_get_list(UNIT_FILE_SYSTEM, root_dir, h);
> > +        assert_se(r >= 0);
> 
> I'm a bit confused here -- is this supposed to have a side effect and
> the condition was rewritten later?

assert_se() is something that is under no conditions optimized
away. This is different from assert() which is removed entirely on
-DNDEBUG. Since these source files are tests, and tests only they
should user assert_se() everywhere, since otherwise they'd lose their
entire purpose on -DNDEBUG.

So, in this case assert_se() vs. assert() is not really a question of
side-effects or not, but more one of "is it ok to remove this line or
not"...

Lennart

-- 
Lennart Poettering, Red Hat