[vlc-devel] [PATCH 2/6] executor: add unit tests

[Romain Vimont]

On Tue, Sep 08, 2020 at 12:06:23PM +0200, Alexandre Janniaux wrote:
> Hi,
> 
> On Mon, Sep 07, 2020 at 07:16:07PM +0200, Romain Vimont wrote:
> > On Mon, Sep 07, 2020 at 06:27:37PM +0200, Alexandre Janniaux wrote:
> > > Hi,
> > >
> > > On Mon, Sep 07, 2020 at 05:40:07PM +0200, Romain Vimont wrote:
> > > > +    /* Wait a bit (in two lines to avoid harmful_delay() warning) */
> > >
> > > To avoid harmful_delay() warnings, you can use (vlc_tick_sleep)(foo);
> >
> > Thank you for the tip.
> >
> > However, I'm not fan of this hack, I find it confusing and it "breaks"
> > tools like cscope/ctags.
> >
> > > > +    vlc_tick_t delay = VLC_TICK_FROM_MS(150);
> > > > +    vlc_tick_sleep(delay);
> 
> Yes, but it could be argued that it's not much prettier
> to hide the warnings by splitting the sleeping calls.
> It's probably better to leave the warning triggers here
> then.

The warning says "use proper event handling instead of short delay":

    ../../src/test/executor.c: In function ‘test_cancel’:
    ../../include/vlc_threads.h:904:14: warning: call to ‘harmful_delay’ declared with attribute warning: use proper event handling instead of short delay [-Wattribute-warning]
      904 |            ? harmful_delay(d) \
          |              ^~~~~~~~~~~~~~~~
    ../../include/vlc_threads.h:923:42: note: in expansion of macro ‘check_delay’
      923 | #define vlc_tick_sleep(d) vlc_tick_sleep(check_delay(d))
          |                                          ^~~~~~~~~~~
    ../../src/test/executor.c:176:5: note: in expansion of macro ‘vlc_tick_sleep’
      176 |     vlc_tick_sleep(VLC_TICK_FROM_MS(150));
          |     ^~~~~~~~~~~~~~

This is a good advice for real code, but not for unit tests (at least
not in this specific case).

> 
> Regards,
> --
> Alexandre Janniaux
> Videolabs
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel