Re: [libgpiod][PATCH] tools: tests: port tests to shunit2

[Kent Gibson <warthog618@xxxxxxxxx>]

On Tue, Jun 20, 2023 at 05:19:27PM +0200, Bartosz Golaszewski wrote:
> On Tue, Jun 20, 2023 at 3:43 PM Kent Gibson <warthog618@xxxxxxxxx> wrote:
> > >
> > >  gpiosim_chip_symlink() {
> > > -     GPIOSIM_CHIP_LINK="$2/${GPIOSIM_APP_NAME}-$$-lnk"
> > > +     GPIOSIM_CHIP_LINK="/tmp/${GPIOSIM_APP_NAME}-$$-lnk"
> > >       ln -s ${GPIOSIM_CHIP_PATH[$1]} "$GPIOSIM_CHIP_LINK"
> > >  }
> >
> > The $2 dicates where the symlink was placed.
> > Now it is ignored and placed in /tmp regardless, which is wrong.
> > Refer to where it is called.
> >
> 
> I understand the $2 was ignored here but why is it wrong to use /tmp?
> Why would we want to create the link in .? Also: shunit defines
> SHUNIT_TMPDIR which seems to be exposed for temporary files generated
> by tests, I'm more inclined towards using this one.
> 

The $2 is there for a reason - that is where you want the symlink
located.

"gpiodetect: all chips" puts a symlink in /dev to check that it is
ignored.

""gpiodetect: a chip" puts one in the PWD to check that gpiodetect will
find it.

If you want to remove that parameter then review and revise  all the
places it is used.

> > > @@ -2072,9 +2063,13 @@ request_release_line() {
> > >       dut_run_redirect gpiomon --num-events=4 --chip $sim0 4
> > >
> > >       gpiosim_set_pull sim0 4 pull-up
> > > +     sleep 0.01
> > >       gpiosim_set_pull sim0 4 pull-down
> > > +     sleep 0.01
> > >       gpiosim_set_pull sim0 4 pull-up
> > > +     sleep 0.01
> > >       gpiosim_set_pull sim0 4 pull-down
> > > +     sleep 0.01
> > >
> >
> > Why are delays now required between sim pulls?
> > Might toggle the pull before it gets propagated to the cdev?
> > Add a function that describes that rather than a raw sleep?
> > gpiosim_wait_pull?
> >
> > Split that out from the shunit2 change if if is a general problem?
> 
> Porting to shunit revealed a problem similar to the one I saw in C
> tests - toggling the sim pull too fast would lead to losing events.
> Turns out BATS was slow enough to hide the problem. If I run shunit
> over strace, the problem is gone too because it slows down the
> execution.
> 

I see the same in some of my compiled unit tests, and add propagation
delays to allow for it.  For me, the tool tests have always been too slow
to trigger it - and still are, but it wouldn't surprise me to see it
on a faster machine.  So no problem with adding some allowance there.

Cheers,
Kent.