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

[Zbigniew Jędrzejewski-Szmek]

On Thu, Oct 23, 2014 at 02:22:01PM +0200, Lennart Poettering wrote:
> 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"...
Except that lines without side effects (like r >= 0)
actually don't do anything if compiled with NDEBUG, no matter if
assert() or assert_se() is used.

Zbyszek