[libgpiod][PATCH 1/1] gpio-tools-test.bats: modify delays in toggle test

[libgpiod][PATCH 1/1] gpio-tools-test.bats: modify delays in toggle test

[joe.slater]

From: Joe Slater <joe.slater@windriver.com>

The test "gpioset: toggle (continuous)" uses fixed delays to test
toggling values.  This is not reliable, so we switch to looking
for transitions from one value to another.

Signed-off-by: Joe Slater <joe.slater@windriver.com>
---
 tools/gpio-tools-test.bats | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/tools/gpio-tools-test.bats b/tools/gpio-tools-test.bats
index adbce94..977d718 100755
--- a/tools/gpio-tools-test.bats
+++ b/tools/gpio-tools-test.bats
@@ -141,6 +141,20 @@ gpiosim_check_value() {
 	[ "$VAL" = "$EXPECTED" ]
 }
 
+gpiosim_wait_value() {
+	local OFFSET=$2
+	local EXPECTED=$3
+	local DEVNAME=${GPIOSIM_DEV_NAME[$1]}
+	local CHIPNAME=${GPIOSIM_CHIP_NAME[$1]}
+
+	for i in {1..10} ; do
+		VAL=$(<$GPIOSIM_SYSFS/$DEVNAME/$CHIPNAME/sim_gpio$OFFSET/value)
+		[ "$VAL" = "$EXPECTED" ] && return
+		sleep 0.1
+	done
+	return 1
+}
+
 gpiosim_cleanup() {
 	for CHIP in ${!GPIOSIM_CHIP_NAME[@]}
 	do
@@ -1567,15 +1581,15 @@ request_release_line() {
 	gpiosim_check_value sim0 4 0
 	gpiosim_check_value sim0 7 0
 
-	sleep 1
-
-	gpiosim_check_value sim0 1 0
+	# sleeping fixed amounts can be unreliable, so we
+	# sync to the toggles
+	#
+	gpiosim_wait_value sim0 1 0
 	gpiosim_check_value sim0 4 1
 	gpiosim_check_value sim0 7 1
 
-	sleep 1
 
-	gpiosim_check_value sim0 1 1
+	gpiosim_wait_value sim0 1 1
 	gpiosim_check_value sim0 4 0
 	gpiosim_check_value sim0 7 0
 }
-- 
2.25.1

Re: [libgpiod][PATCH 1/1] gpio-tools-test.bats: modify delays in toggle test

[Kent Gibson]

On Wed, May 24, 2023 at 02:09:45PM -0700, joe.slater@windriver.com wrote:
> From: Joe Slater <joe.slater@windriver.com>
> 
> The test "gpioset: toggle (continuous)" uses fixed delays to test
> toggling values.  This is not reliable, so we switch to looking
> for transitions from one value to another.
> 

That test is prone to spurious failures if either the test or gpioset
get delayed for any reason, so good idea.

> Signed-off-by: Joe Slater <joe.slater@windriver.com>
> ---
>  tools/gpio-tools-test.bats | 24 +++++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/gpio-tools-test.bats b/tools/gpio-tools-test.bats
> index adbce94..977d718 100755
> --- a/tools/gpio-tools-test.bats
> +++ b/tools/gpio-tools-test.bats
> @@ -141,6 +141,20 @@ gpiosim_check_value() {
>  	[ "$VAL" = "$EXPECTED" ]
>  }
>  
> +gpiosim_wait_value() {
> +	local OFFSET=$2
> +	local EXPECTED=$3
> +	local DEVNAME=${GPIOSIM_DEV_NAME[$1]}
> +	local CHIPNAME=${GPIOSIM_CHIP_NAME[$1]}
> +
> +	for i in {1..10} ; do
> +		VAL=$(<$GPIOSIM_SYSFS/$DEVNAME/$CHIPNAME/sim_gpio$OFFSET/value)
> +		[ "$VAL" = "$EXPECTED" ] && return
> +		sleep 0.1
> +	done
> +	return 1
> +}
> +

Not a huge fan of the loop limit and sleep period being hard coded,
as those control the time to wait, but as it is only used in the one
place I can live with it.

>  gpiosim_cleanup() {
>  	for CHIP in ${!GPIOSIM_CHIP_NAME[@]}
>  	do
> @@ -1567,15 +1581,15 @@ request_release_line() {
>  	gpiosim_check_value sim0 4 0
>  	gpiosim_check_value sim0 7 0
>  
> -	sleep 1
> -
> -	gpiosim_check_value sim0 1 0
> +	# sleeping fixed amounts can be unreliable, so we
> +	# sync to the toggles
> +	#
> +	gpiosim_wait_value sim0 1 0
>  	gpiosim_check_value sim0 4 1
>  	gpiosim_check_value sim0 7 1
>  

The comment is confusing once the sleep is removed, so just drop it.
If you want to describe what gpiosim_wait_value() does and when it should
be used then add that before the function itself.

The test toggles the line at 1s intervals to try to improve the
chances of the test and gpioset staying in sync.
Could that be reduced now, without impacting reliability?
(this test suite being glacial is a personal bugbear)

Cheers,
Kent.

RE: [libgpiod][PATCH 1/1] gpio-tools-test.bats: modify delays in toggle test

[Slater, Joseph]

> -----Original Message-----
> From: Kent Gibson <warthog618@gmail.com>
> Sent: Wednesday, May 24, 2023 8:53 PM
> To: Slater, Joseph <joe.slater@windriver.com>
> Cc: linux-gpio@vger.kernel.org; MacLeod, Randy
> <Randy.MacLeod@windriver.com>
> Subject: Re: [libgpiod][PATCH 1/1] gpio-tools-test.bats: modify delays in toggle
> test
> 
> On Wed, May 24, 2023 at 02:09:45PM -0700, joe.slater@windriver.com wrote:
> > From: Joe Slater <joe.slater@windriver.com>
> >
> > The test "gpioset: toggle (continuous)" uses fixed delays to test
> > toggling values.  This is not reliable, so we switch to looking for
> > transitions from one value to another.
> >
> 
> That test is prone to spurious failures if either the test or gpioset get delayed for
> any reason, so good idea.
> 
> > Signed-off-by: Joe Slater <joe.slater@windriver.com>
> > ---
> >  tools/gpio-tools-test.bats | 24 +++++++++++++++++++-----
> >  1 file changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/gpio-tools-test.bats b/tools/gpio-tools-test.bats
> > index adbce94..977d718 100755
> > --- a/tools/gpio-tools-test.bats
> > +++ b/tools/gpio-tools-test.bats
> > @@ -141,6 +141,20 @@ gpiosim_check_value() {
> >  	[ "$VAL" = "$EXPECTED" ]
> >  }
> >
> > +gpiosim_wait_value() {
> > +	local OFFSET=$2
> > +	local EXPECTED=$3
> > +	local DEVNAME=${GPIOSIM_DEV_NAME[$1]}
> > +	local CHIPNAME=${GPIOSIM_CHIP_NAME[$1]}
> > +
> > +	for i in {1..10} ; do
> > +
> 	VAL=$(<$GPIOSIM_SYSFS/$DEVNAME/$CHIPNAME/sim_gpio$OFFSET/v
> alue)
> > +		[ "$VAL" = "$EXPECTED" ] && return
> > +		sleep 0.1
> > +	done
> > +	return 1
> > +}
> > +
> 
> Not a huge fan of the loop limit and sleep period being hard coded, as those
> control the time to wait, but as it is only used in the one place I can live with it.
> 
> >  gpiosim_cleanup() {
> >  	for CHIP in ${!GPIOSIM_CHIP_NAME[@]}
> >  	do
> > @@ -1567,15 +1581,15 @@ request_release_line() {
> >  	gpiosim_check_value sim0 4 0
> >  	gpiosim_check_value sim0 7 0
> >
> > -	sleep 1
> > -
> > -	gpiosim_check_value sim0 1 0
> > +	# sleeping fixed amounts can be unreliable, so we
> > +	# sync to the toggles
> > +	#
> > +	gpiosim_wait_value sim0 1 0
> >  	gpiosim_check_value sim0 4 1
> >  	gpiosim_check_value sim0 7 1
> >
> 
> The comment is confusing once the sleep is removed, so just drop it.
> If you want to describe what gpiosim_wait_value() does and when it should be
> used then add that before the function itself.
> 
> The test toggles the line at 1s intervals to try to improve the chances of the test
> and gpioset staying in sync.
> Could that be reduced now, without impacting reliability?
> (this test suite being glacial is a personal bugbear)

[Slater, Joseph] I'll get rid of the comment and try the test with a shorter toggle time.
The series of 159 tests takes, maybe, 10-15 minutes for me, so I don't think saving a
second or two here would make much difference, though.
Joe

> 
> Cheers,
> Kent.

Re: [libgpiod][PATCH 1/1] gpio-tools-test.bats: modify delays in toggle test

[Kent Gibson]

On Thu, May 25, 2023 at 09:54:14PM +0000, Slater, Joseph wrote:
> 
> 
> > -----Original Message-----
> > From: Kent Gibson <warthog618@gmail.com>
> > Sent: Wednesday, May 24, 2023 8:53 PM
> > To: Slater, Joseph <joe.slater@windriver.com>
> > Cc: linux-gpio@vger.kernel.org; MacLeod, Randy
> > <Randy.MacLeod@windriver.com>
> > Subject: Re: [libgpiod][PATCH 1/1] gpio-tools-test.bats: modify delays in toggle
> > test
> > 
> > The comment is confusing once the sleep is removed, so just drop it.
> > If you want to describe what gpiosim_wait_value() does and when it should be
> > used then add that before the function itself.
> > 
> > The test toggles the line at 1s intervals to try to improve the chances of the test
> > and gpioset staying in sync.
> > Could that be reduced now, without impacting reliability?
> > (this test suite being glacial is a personal bugbear)
> 
> [Slater, Joseph] I'll get rid of the comment and try the test with a shorter toggle time.
> The series of 159 tests takes, maybe, 10-15 minutes for me, so I don't think saving a
> second or two here would make much difference, though.
> Joe
> 

I did mention it was glacial, so I feel your pain - you have no idea how
many times I've run that test suite - though it is a bit quicker than
10-15 for me.

So no problem with leaving the timings as is. I can look at it some
other time - revisiting that test suite to try to speed it up is on my
todo list - somewhere near the bottom.

I look forward to v2.

Cheers,
Kent.

Re: [libgpiod][PATCH 1/1] gpio-tools-test.bats: modify delays in toggle test

[Bartosz Golaszewski]

On Thu, May 25, 2023 at 11:54 PM Slater, Joseph
<joe.slater@windriver.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Kent Gibson <warthog618@gmail.com>
> > Sent: Wednesday, May 24, 2023 8:53 PM
> > To: Slater, Joseph <joe.slater@windriver.com>
> > Cc: linux-gpio@vger.kernel.org; MacLeod, Randy
> > <Randy.MacLeod@windriver.com>
> > Subject: Re: [libgpiod][PATCH 1/1] gpio-tools-test.bats: modify delays in toggle
> > test
> >
> > On Wed, May 24, 2023 at 02:09:45PM -0700, joe.slater@windriver.com wrote:
> > > From: Joe Slater <joe.slater@windriver.com>
> > >
> > > The test "gpioset: toggle (continuous)" uses fixed delays to test
> > > toggling values.  This is not reliable, so we switch to looking for
> > > transitions from one value to another.
> > >
> >
> > That test is prone to spurious failures if either the test or gpioset get delayed for
> > any reason, so good idea.
> >
> > > Signed-off-by: Joe Slater <joe.slater@windriver.com>
> > > ---
> > >  tools/gpio-tools-test.bats | 24 +++++++++++++++++++-----
> > >  1 file changed, 19 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/tools/gpio-tools-test.bats b/tools/gpio-tools-test.bats
> > > index adbce94..977d718 100755
> > > --- a/tools/gpio-tools-test.bats
> > > +++ b/tools/gpio-tools-test.bats
> > > @@ -141,6 +141,20 @@ gpiosim_check_value() {
> > >     [ "$VAL" = "$EXPECTED" ]
> > >  }
> > >
> > > +gpiosim_wait_value() {
> > > +   local OFFSET=$2
> > > +   local EXPECTED=$3
> > > +   local DEVNAME=${GPIOSIM_DEV_NAME[$1]}
> > > +   local CHIPNAME=${GPIOSIM_CHIP_NAME[$1]}
> > > +
> > > +   for i in {1..10} ; do
> > > +
> >       VAL=$(<$GPIOSIM_SYSFS/$DEVNAME/$CHIPNAME/sim_gpio$OFFSET/v
> > alue)
> > > +           [ "$VAL" = "$EXPECTED" ] && return
> > > +           sleep 0.1
> > > +   done
> > > +   return 1
> > > +}
> > > +
> >
> > Not a huge fan of the loop limit and sleep period being hard coded, as those
> > control the time to wait, but as it is only used in the one place I can live with it.
> >
> > >  gpiosim_cleanup() {
> > >     for CHIP in ${!GPIOSIM_CHIP_NAME[@]}
> > >     do
> > > @@ -1567,15 +1581,15 @@ request_release_line() {
> > >     gpiosim_check_value sim0 4 0
> > >     gpiosim_check_value sim0 7 0
> > >
> > > -   sleep 1
> > > -
> > > -   gpiosim_check_value sim0 1 0
> > > +   # sleeping fixed amounts can be unreliable, so we
> > > +   # sync to the toggles
> > > +   #
> > > +   gpiosim_wait_value sim0 1 0
> > >     gpiosim_check_value sim0 4 1
> > >     gpiosim_check_value sim0 7 1
> > >
> >
> > The comment is confusing once the sleep is removed, so just drop it.
> > If you want to describe what gpiosim_wait_value() does and when it should be
> > used then add that before the function itself.
> >
> > The test toggles the line at 1s intervals to try to improve the chances of the test
> > and gpioset staying in sync.
> > Could that be reduced now, without impacting reliability?
> > (this test suite being glacial is a personal bugbear)
>
> [Slater, Joseph] I'll get rid of the comment and try the test with a shorter toggle time.
> The series of 159 tests takes, maybe, 10-15 minutes for me, so I don't think saving a
> second or two here would make much difference, though.
> Joe
>

That can't be right, are you running it on a toaster? It takes 26
seconds on my regular lenovo thinkpad laptop which is still longer
than I'd like but quite acceptable for now (though I agree it would be
great to improve it).

Bart

Re: [libgpiod][PATCH 1/1] gpio-tools-test.bats: modify delays in toggle test

[Kent Gibson]

On Tue, May 30, 2023 at 11:51:05AM +0200, Bartosz Golaszewski wrote:
> On Thu, May 25, 2023 at 11:54 PM Slater, Joseph
> <joe.slater@windriver.com> wrote:
> >
> >
> > [Slater, Joseph] I'll get rid of the comment and try the test with a shorter toggle time.
> > The series of 159 tests takes, maybe, 10-15 minutes for me, so I don't think saving a
> > second or two here would make much difference, though.
> > Joe
> >
> 
> That can't be right, are you running it on a toaster? It takes 26
> seconds on my regular lenovo thinkpad laptop which is still longer
> than I'd like but quite acceptable for now (though I agree it would be
> great to improve it).
> 

Consider yourself blessed.
I just got:

real	2m25.956s

on my test VM.
Not sure exactly what the hold up is - it isn't using much CPU, or any
other resources AFAICT.
Compared to all the other test suites I run, this is far and away the
slowest, especially since switching everything over to gpio-sim.

Cheers,
Kent.

Re: [libgpiod][PATCH 1/1] gpio-tools-test.bats: modify delays in toggle test

[Bartosz Golaszewski]

On Tue, May 30, 2023 at 12:05 PM Kent Gibson <warthog618@gmail.com> wrote:
>
> On Tue, May 30, 2023 at 11:51:05AM +0200, Bartosz Golaszewski wrote:
> > On Thu, May 25, 2023 at 11:54 PM Slater, Joseph
> > <joe.slater@windriver.com> wrote:
> > >
> > >
> > > [Slater, Joseph] I'll get rid of the comment and try the test with a shorter toggle time.
> > > The series of 159 tests takes, maybe, 10-15 minutes for me, so I don't think saving a
> > > second or two here would make much difference, though.
> > > Joe
> > >
> >
> > That can't be right, are you running it on a toaster? It takes 26
> > seconds on my regular lenovo thinkpad laptop which is still longer
> > than I'd like but quite acceptable for now (though I agree it would be
> > great to improve it).
> >
>
> Consider yourself blessed.
> I just got:
>
> real    2m25.956s
>
> on my test VM.
> Not sure exactly what the hold up is - it isn't using much CPU, or any
> other resources AFAICT.
> Compared to all the other test suites I run, this is far and away the
> slowest, especially since switching everything over to gpio-sim.

I agree it's too slow - be it 20 seconds or 2 minutes. But similarly
to you - it's very low on my TODO list as I only run it every once in
a while. I'd be happy to accept any patches improving the situation of
course.

Bart

Re: [libgpiod][PATCH 1/1] gpio-tools-test.bats: modify delays in toggle test

[Kent Gibson]

On Tue, May 30, 2023 at 04:13:06PM +0200, Bartosz Golaszewski wrote:
> On Tue, May 30, 2023 at 12:05 PM Kent Gibson <warthog618@gmail.com> wrote:
> >
> > On Tue, May 30, 2023 at 11:51:05AM +0200, Bartosz Golaszewski wrote:
> > > On Thu, May 25, 2023 at 11:54 PM Slater, Joseph
> > > <joe.slater@windriver.com> wrote:
> > > >
> > > >
> > > > [Slater, Joseph] I'll get rid of the comment and try the test with a shorter toggle time.
> > > > The series of 159 tests takes, maybe, 10-15 minutes for me, so I don't think saving a
> > > > second or two here would make much difference, though.
> > > > Joe
> > > >
> > >
> > > That can't be right, are you running it on a toaster? It takes 26
> > > seconds on my regular lenovo thinkpad laptop which is still longer
> > > than I'd like but quite acceptable for now (though I agree it would be
> > > great to improve it).
> > >
> >
> > Consider yourself blessed.
> > I just got:
> >
> > real    2m25.956s
> >
> > on my test VM.
> > Not sure exactly what the hold up is - it isn't using much CPU, or any
> > other resources AFAICT.
> > Compared to all the other test suites I run, this is far and away the
> > slowest, especially since switching everything over to gpio-sim.
> 
> I agree it's too slow - be it 20 seconds or 2 minutes. But similarly
> to you - it's very low on my TODO list as I only run it every once in
> a while. I'd be happy to accept any patches improving the situation of
> course.
> 

Same.  I already had a go at streamlining the tests when I updated them
for v2, so it is somewhat better than it was, but it is still painfully
slow for me.
To improve further I'd have to start digging around to see what bats is
up to.  Speaking of which, do we need to stick with bats?
I've driven similar tests with Python in the past, and I'm sure that
would provide a better experience.
What constraints do we have?

Cheers,
Kent.

Re: [libgpiod][PATCH 1/1] gpio-tools-test.bats: modify delays in toggle test

[Bartosz Golaszewski]

On Tue, May 30, 2023 at 4:24 PM Kent Gibson <warthog618@gmail.com> wrote:
>
> On Tue, May 30, 2023 at 04:13:06PM +0200, Bartosz Golaszewski wrote:
> > On Tue, May 30, 2023 at 12:05 PM Kent Gibson <warthog618@gmail.com> wrote:
> > >
> > > On Tue, May 30, 2023 at 11:51:05AM +0200, Bartosz Golaszewski wrote:
> > > > On Thu, May 25, 2023 at 11:54 PM Slater, Joseph
> > > > <joe.slater@windriver.com> wrote:
> > > > >
> > > > >
> > > > > [Slater, Joseph] I'll get rid of the comment and try the test with a shorter toggle time.
> > > > > The series of 159 tests takes, maybe, 10-15 minutes for me, so I don't think saving a
> > > > > second or two here would make much difference, though.
> > > > > Joe
> > > > >
> > > >
> > > > That can't be right, are you running it on a toaster? It takes 26
> > > > seconds on my regular lenovo thinkpad laptop which is still longer
> > > > than I'd like but quite acceptable for now (though I agree it would be
> > > > great to improve it).
> > > >
> > >
> > > Consider yourself blessed.
> > > I just got:
> > >
> > > real    2m25.956s
> > >
> > > on my test VM.
> > > Not sure exactly what the hold up is - it isn't using much CPU, or any
> > > other resources AFAICT.
> > > Compared to all the other test suites I run, this is far and away the
> > > slowest, especially since switching everything over to gpio-sim.
> >
> > I agree it's too slow - be it 20 seconds or 2 minutes. But similarly
> > to you - it's very low on my TODO list as I only run it every once in
> > a while. I'd be happy to accept any patches improving the situation of
> > course.
> >
>
> Same.  I already had a go at streamlining the tests when I updated them
> for v2, so it is somewhat better than it was, but it is still painfully
> slow for me.
> To improve further I'd have to start digging around to see what bats is
> up to.  Speaking of which, do we need to stick with bats?
> I've driven similar tests with Python in the past, and I'm sure that
> would provide a better experience.
> What constraints do we have?
>

I went with bats because it looked the fastest to write tests in -
it's shell after all.

But I think that we could potentially package whatever python
subprocess code we need into enough helper wrappers that it wouldn't
be much more complex than this.

We also already have python wrappers for libgpiosim ready.

No objections from my side, it's just that I won't have time to
rewrite the entire thing in Python anytime soon.

Bartosz

Re: [libgpiod][PATCH 1/1] gpio-tools-test.bats: modify delays in toggle test

[Kent Gibson]

On Tue, May 30, 2023 at 04:52:36PM +0200, Bartosz Golaszewski wrote:
> On Tue, May 30, 2023 at 4:24 PM Kent Gibson <warthog618@gmail.com> wrote:
> >
> > On Tue, May 30, 2023 at 04:13:06PM +0200, Bartosz Golaszewski wrote:
> > > On Tue, May 30, 2023 at 12:05 PM Kent Gibson <warthog618@gmail.com> wrote:
> > > >
> > > > On Tue, May 30, 2023 at 11:51:05AM +0200, Bartosz Golaszewski wrote:
> > > > > On Thu, May 25, 2023 at 11:54 PM Slater, Joseph
> > > > > <joe.slater@windriver.com> wrote:
> > > > > >
> > > > > >
> > > > > > [Slater, Joseph] I'll get rid of the comment and try the test with a shorter toggle time.
> > > > > > The series of 159 tests takes, maybe, 10-15 minutes for me, so I don't think saving a
> > > > > > second or two here would make much difference, though.
> > > > > > Joe
> > > > > >
> > > > >
> > > > > That can't be right, are you running it on a toaster? It takes 26
> > > > > seconds on my regular lenovo thinkpad laptop which is still longer
> > > > > than I'd like but quite acceptable for now (though I agree it would be
> > > > > great to improve it).
> > > > >
> > > >
> > > > Consider yourself blessed.
> > > > I just got:
> > > >
> > > > real    2m25.956s
> > > >
> > > > on my test VM.
> > > > Not sure exactly what the hold up is - it isn't using much CPU, or any
> > > > other resources AFAICT.
> > > > Compared to all the other test suites I run, this is far and away the
> > > > slowest, especially since switching everything over to gpio-sim.
> > >
> > > I agree it's too slow - be it 20 seconds or 2 minutes. But similarly
> > > to you - it's very low on my TODO list as I only run it every once in
> > > a while. I'd be happy to accept any patches improving the situation of
> > > course.
> > >
> >
> > Same.  I already had a go at streamlining the tests when I updated them
> > for v2, so it is somewhat better than it was, but it is still painfully
> > slow for me.
> > To improve further I'd have to start digging around to see what bats is
> > up to.  Speaking of which, do we need to stick with bats?
> > I've driven similar tests with Python in the past, and I'm sure that
> > would provide a better experience.
> > What constraints do we have?
> >
> 
> I went with bats because it looked the fastest to write tests in -
> it's shell after all.
> 

Really?  I wouldn't write anything of consequence in shell if Python was
an option.

How about Rust?  I've gotten over how spartan the Rust test framework is
so I wouldn't have a problem writing it in that either.

> But I think that we could potentially package whatever python
> subprocess code we need into enough helper wrappers that it wouldn't
> be much more complex than this.
> 

The end result would look similar in terms of test complexity, but
should be much easier to write and debug.
Not that that counts for much given we have a functional bats test
suite.

> We also already have python wrappers for libgpiosim ready.
> 

Exactly.  And Rust bindings too.

> No objections from my side, it's just that I won't have time to
> rewrite the entire thing in Python anytime soon.
> 

I'll update my todo list.  No promises - it is low priority given the
only real problem with the bats solution is its performance.

Cheers,
Kent.

Re: [libgpiod][PATCH 1/1] gpio-tools-test.bats: modify delays in toggle test

[Bartosz Golaszewski]

On Tue, May 30, 2023 at 5:18 PM Kent Gibson <warthog618@gmail.com> wrote:
>
> On Tue, May 30, 2023 at 04:52:36PM +0200, Bartosz Golaszewski wrote:
> > On Tue, May 30, 2023 at 4:24 PM Kent Gibson <warthog618@gmail.com> wrote:
> > >
> > > On Tue, May 30, 2023 at 04:13:06PM +0200, Bartosz Golaszewski wrote:
> > > > On Tue, May 30, 2023 at 12:05 PM Kent Gibson <warthog618@gmail.com> wrote:
> > > > >
> > > > > On Tue, May 30, 2023 at 11:51:05AM +0200, Bartosz Golaszewski wrote:
> > > > > > On Thu, May 25, 2023 at 11:54 PM Slater, Joseph
> > > > > > <joe.slater@windriver.com> wrote:
> > > > > > >
> > > > > > >
> > > > > > > [Slater, Joseph] I'll get rid of the comment and try the test with a shorter toggle time.
> > > > > > > The series of 159 tests takes, maybe, 10-15 minutes for me, so I don't think saving a
> > > > > > > second or two here would make much difference, though.
> > > > > > > Joe
> > > > > > >
> > > > > >
> > > > > > That can't be right, are you running it on a toaster? It takes 26
> > > > > > seconds on my regular lenovo thinkpad laptop which is still longer
> > > > > > than I'd like but quite acceptable for now (though I agree it would be
> > > > > > great to improve it).
> > > > > >
> > > > >
> > > > > Consider yourself blessed.
> > > > > I just got:
> > > > >
> > > > > real    2m25.956s
> > > > >
> > > > > on my test VM.
> > > > > Not sure exactly what the hold up is - it isn't using much CPU, or any
> > > > > other resources AFAICT.
> > > > > Compared to all the other test suites I run, this is far and away the
> > > > > slowest, especially since switching everything over to gpio-sim.
> > > >
> > > > I agree it's too slow - be it 20 seconds or 2 minutes. But similarly
> > > > to you - it's very low on my TODO list as I only run it every once in
> > > > a while. I'd be happy to accept any patches improving the situation of
> > > > course.
> > > >
> > >
> > > Same.  I already had a go at streamlining the tests when I updated them
> > > for v2, so it is somewhat better than it was, but it is still painfully
> > > slow for me.
> > > To improve further I'd have to start digging around to see what bats is
> > > up to.  Speaking of which, do we need to stick with bats?
> > > I've driven similar tests with Python in the past, and I'm sure that
> > > would provide a better experience.
> > > What constraints do we have?
> > >
> >
> > I went with bats because it looked the fastest to write tests in -
> > it's shell after all.
> >
>
> Really?  I wouldn't write anything of consequence in shell if Python was
> an option.
>
> How about Rust?  I've gotten over how spartan the Rust test framework is
> so I wouldn't have a problem writing it in that either.
>

I have a very strong preference for Python. I am quite bad at Rust.
Whatever is in bindings/rust/ is Viresh' jurisdiction and I defer to
him but I would prefer to be able to keep track of what's happening in
tools/ and work on it myself without too much frustration. And writing
anything in rust has been pure frustration so far.

Bartosz

> > But I think that we could potentially package whatever python
> > subprocess code we need into enough helper wrappers that it wouldn't
> > be much more complex than this.
> >
>
> The end result would look similar in terms of test complexity, but
> should be much easier to write and debug.
> Not that that counts for much given we have a functional bats test
> suite.
>
> > We also already have python wrappers for libgpiosim ready.
> >
>
> Exactly.  And Rust bindings too.
>
> > No objections from my side, it's just that I won't have time to
> > rewrite the entire thing in Python anytime soon.
> >
>
> I'll update my todo list.  No promises - it is low priority given the
> only real problem with the bats solution is its performance.
>
> Cheers,
> Kent.
>

Re: [libgpiod][PATCH 1/1] gpio-tools-test.bats: modify delays in toggle test

[Kent Gibson]

On Tue, May 30, 2023 at 06:07:52PM +0200, Bartosz Golaszewski wrote:
> On Tue, May 30, 2023 at 5:18 PM Kent Gibson <warthog618@gmail.com> wrote:
> >
> > On Tue, May 30, 2023 at 04:52:36PM +0200, Bartosz Golaszewski wrote:
> > > On Tue, May 30, 2023 at 4:24 PM Kent Gibson <warthog618@gmail.com> wrote:
> > > >
> > > > On Tue, May 30, 2023 at 04:13:06PM +0200, Bartosz Golaszewski wrote:
> > > > > On Tue, May 30, 2023 at 12:05 PM Kent Gibson <warthog618@gmail.com> wrote:
> > >
> > > I went with bats because it looked the fastest to write tests in -
> > > it's shell after all.
> > >
> >
> > Really?  I wouldn't write anything of consequence in shell if Python was
> > an option.
> >
> > How about Rust?  I've gotten over how spartan the Rust test framework is
> > so I wouldn't have a problem writing it in that either.
> >
> 
> I have a very strong preference for Python. I am quite bad at Rust.
> Whatever is in bindings/rust/ is Viresh' jurisdiction and I defer to
> him but I would prefer to be able to keep track of what's happening in
> tools/ and work on it myself without too much frustration. And writing
> anything in rust has been pure frustration so far.
> 

Fair enough, Python it is then.

I personally had no problem picking up Rust - seems Rust and I have a
similar view - I've always had issues with the vagueness of ownership
and lifetimes in other languages, particularly C/C++.  Rust gets it.
And if you do make a hash of something clippy provides good suggestions,
or at least clearly identifies the problem. That helped me a lot with
the learning curve.

Cheers,
Kent.

Re: [libgpiod][PATCH 1/1] gpio-tools-test.bats: modify delays in toggle test

[Bartosz Golaszewski]

On Wed, May 31, 2023 at 3:17 AM Kent Gibson <warthog618@gmail.com> wrote:
>
> On Tue, May 30, 2023 at 06:07:52PM +0200, Bartosz Golaszewski wrote:
> > On Tue, May 30, 2023 at 5:18 PM Kent Gibson <warthog618@gmail.com> wrote:
> > >
> > > On Tue, May 30, 2023 at 04:52:36PM +0200, Bartosz Golaszewski wrote:
> > > > On Tue, May 30, 2023 at 4:24 PM Kent Gibson <warthog618@gmail.com> wrote:
> > > > >
> > > > > On Tue, May 30, 2023 at 04:13:06PM +0200, Bartosz Golaszewski wrote:
> > > > > > On Tue, May 30, 2023 at 12:05 PM Kent Gibson <warthog618@gmail.com> wrote:
> > > >
> > > > I went with bats because it looked the fastest to write tests in -
> > > > it's shell after all.
> > > >
> > >
> > > Really?  I wouldn't write anything of consequence in shell if Python was
> > > an option.
> > >
> > > How about Rust?  I've gotten over how spartan the Rust test framework is
> > > so I wouldn't have a problem writing it in that either.
> > >
> >
> > I have a very strong preference for Python. I am quite bad at Rust.
> > Whatever is in bindings/rust/ is Viresh' jurisdiction and I defer to
> > him but I would prefer to be able to keep track of what's happening in
> > tools/ and work on it myself without too much frustration. And writing
> > anything in rust has been pure frustration so far.
> >
>
> Fair enough, Python it is then.
>
> I personally had no problem picking up Rust - seems Rust and I have a
> similar view - I've always had issues with the vagueness of ownership
> and lifetimes in other languages, particularly C/C++.  Rust gets it.
> And if you do make a hash of something clippy provides good suggestions,
> or at least clearly identifies the problem. That helped me a lot with
> the learning curve.
>
> Cheers,
> Kent.

Before jumping into a complete rewrite, I thought it's worth at least
giving bats a chance and see if we can simply fix the delay. A quick
strace run is telling me this:

% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ------------------
 91,93   20,569295        1014     20284      8653 wait4
  1,72    0,384721           0    814827           rt_sigprocmask
  1,35    0,301451           1    171558    142755 readlink
  1,19    0,265404           1    261357         8 read
  1,04    0,233103          20     11283           clone
  0,50    0,110845          16      6848      1025 execve
  0,49    0,110042           0    128382     13692 newfstatat
  0,33    0,073843           0     95369     49559 ioctl
  0,25    0,055440           0     78664           mmap
  0,20    0,044861           2     22081           write
[...]

It suggests, there's some issue with waiting for exiting processes
that if we fix, we should decrease the test time by at least 90%.

Bart

Re: [libgpiod][PATCH 1/1] gpio-tools-test.bats: modify delays in toggle test

[Bartosz Golaszewski]

On Thu, Jun 1, 2023 at 3:16 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> On Wed, May 31, 2023 at 3:17 AM Kent Gibson <warthog618@gmail.com> wrote:
> >
> > On Tue, May 30, 2023 at 06:07:52PM +0200, Bartosz Golaszewski wrote:
> > > On Tue, May 30, 2023 at 5:18 PM Kent Gibson <warthog618@gmail.com> wrote:
> > > >
> > > > On Tue, May 30, 2023 at 04:52:36PM +0200, Bartosz Golaszewski wrote:
> > > > > On Tue, May 30, 2023 at 4:24 PM Kent Gibson <warthog618@gmail.com> wrote:
> > > > > >
> > > > > > On Tue, May 30, 2023 at 04:13:06PM +0200, Bartosz Golaszewski wrote:
> > > > > > > On Tue, May 30, 2023 at 12:05 PM Kent Gibson <warthog618@gmail.com> wrote:
> > > > >
> > > > > I went with bats because it looked the fastest to write tests in -
> > > > > it's shell after all.
> > > > >
> > > >
> > > > Really?  I wouldn't write anything of consequence in shell if Python was
> > > > an option.
> > > >
> > > > How about Rust?  I've gotten over how spartan the Rust test framework is
> > > > so I wouldn't have a problem writing it in that either.
> > > >
> > >
> > > I have a very strong preference for Python. I am quite bad at Rust.
> > > Whatever is in bindings/rust/ is Viresh' jurisdiction and I defer to
> > > him but I would prefer to be able to keep track of what's happening in
> > > tools/ and work on it myself without too much frustration. And writing
> > > anything in rust has been pure frustration so far.
> > >
> >
> > Fair enough, Python it is then.
> >
> > I personally had no problem picking up Rust - seems Rust and I have a
> > similar view - I've always had issues with the vagueness of ownership
> > and lifetimes in other languages, particularly C/C++.  Rust gets it.
> > And if you do make a hash of something clippy provides good suggestions,
> > or at least clearly identifies the problem. That helped me a lot with
> > the learning curve.
> >
> > Cheers,
> > Kent.
>
> Before jumping into a complete rewrite, I thought it's worth at least
> giving bats a chance and see if we can simply fix the delay. A quick
> strace run is telling me this:
>
> % time     seconds  usecs/call     calls    errors syscall
> ------ ----------- ----------- --------- --------- ------------------
>  91,93   20,569295        1014     20284      8653 wait4
>   1,72    0,384721           0    814827           rt_sigprocmask
>   1,35    0,301451           1    171558    142755 readlink
>   1,19    0,265404           1    261357         8 read
>   1,04    0,233103          20     11283           clone
>   0,50    0,110845          16      6848      1025 execve
>   0,49    0,110042           0    128382     13692 newfstatat
>   0,33    0,073843           0     95369     49559 ioctl
>   0,25    0,055440           0     78664           mmap
>   0,20    0,044861           2     22081           write
> [...]
>
> It suggests, there's some issue with waiting for exiting processes
> that if we fix, we should decrease the test time by at least 90%.
>
> Bart

Nevermind that, turns out strace -c -f doesn't do what I thought it
does. It only shows the stats for the top process which mostly just
waits for other processes. That's not the culprit. I need to figure
out a way to correctly profile this.

Bart