Re: [yocto] [ptest-runner] Run ptests via stdbuf configured to line-buffering
On Fri, Apr 05, 2019 at 07:59:41AM +0100, richard.pur...@linuxfoundation.org wrote: > On Fri, 2019-04-05 at 06:54 +, mikko.rap...@bmw.de wrote: > > On Fri, Apr 05, 2019 at 07:46:00AM +0100, > > richard.pur...@linuxfoundation.org wrote: > > > On Fri, 2019-04-05 at 06:16 +, mikko.rap...@bmw.de wrote: > > > > On Thu, Apr 04, 2019 at 10:48:17PM +0100, Richard Purdie wrote: > > > > > On Thu, 2019-04-04 at 18:00 +0200, Alexander Kanavin wrote: > > > > > > As ptest-runner communicates with child processes via > > > > > > pipe2(), > > > > > > the corresponding channels are not attached to a pty. In that > > > > > > situation stdio facilities like printf() or fwrite() are > > > > > > fully > > > > > > buffered. If a ptest would use them, without bothering > > > > > > to fflush() the output, ptest-runner will only receive what > > > > > > was written by the child ptest process after a buffer gets > > > > > > filled. > > > > > > If the unit tests are proceeding slowly, this may mean that > > > > > > ptest-runner will erroneously timeout due to an apparent lack > > > > > > of > > > > > > 'signs of life' from the child process. > > > > > > > > > > > > stdbuf utility from coreutils adjusts the buffering to a > > > > > > line- > > > > > > buffered > > > > > > one, and so ptest-runner will get the lines as soon as they > > > > > > are > > > > > > written. > > > > > > > > > > > > Signed-off-by: Alexander Kanavin > > > > > > --- > > > > > > utils.c | 7 ++- > > > > > > 1 file changed, 2 insertions(+), 5 deletions(-) > > > > > > > > > > I'm a little torn on this. I noticed some of the run-ptest > > > > > scripts > > > > > use > > > > > "| sed -u" whilst the one you were seeing problems with uses "| > > > > > sed" > > > > > without -u. > > > > > > > > > > We may want to consider strongly recommending -u. I'm testing a > > > > > patch > > > > > with some tweaks like that in it... > > > > > > > > Please no. I'm running images without sed and using busybox sed > > > > instead, and that > > > > doesn't support -u. I'd rather be compatible with sed from > > > > busybox to > > > > keep changes > > > > to images minimal (e.g. install of additional packages) before > > > > executing ptests. > > > > > > The other alternative option being proposed is for ptest-runner to > > > depend on coreutils which is worse? > > > > GNU sed does not come from coreutils but from sed recipe. > > > > Your call in the end. I just provided my point of view. > > The original patch in this thread from Alex needs stdbuf which would > add a coreutils depends to ptest-runner for everything as I understand > it. Ok, that too then. Then all hope is lost so lets include sed there as well :) > > > I did test the -u option to sed in the openssh ptest runner and it > > > did > > > fix the problems we've been seeing. > > > > > > I'm open to other alternatives but the -u option to sed is looking > > > like > > > the best one we have right now. These bugs are making our testing > > > of > > > ptests effectively useless and unpredictable so this is a serious > > > issue... > > > > Understood. I hope you could also add 'set -eux' to all ptest shell > > scripts. Many of them seem to be missing shell script error handling > > and failures like providing -u to busybox sed may go unnoticed. > > Patches would be welcome to help clean up some of these scripts. I feel > like I'm fighting a lonely battle trying to get any of this to work at > times :( We're ramping up with ptests and will provide fixes back to upstream. > The good news is the automated monitoring of the ptest results should > catch problems like that as if a chunk of ptests "disappear", the new > testing should quickly highlight that. > > (We should improve the scripts, we just also have another way to spot > problems) All this work is very much appreciated here in downstream! Thanks you very much! -Mikko -- ___ yocto mailing list yocto@yoctoproject.org https://lists.yoctoproject.org/listinfo/yocto
Re: [yocto] [ptest-runner] Run ptests via stdbuf configured to line-buffering
On Fri, 2019-04-05 at 06:54 +, mikko.rap...@bmw.de wrote: > On Fri, Apr 05, 2019 at 07:46:00AM +0100, > richard.pur...@linuxfoundation.org wrote: > > On Fri, 2019-04-05 at 06:16 +, mikko.rap...@bmw.de wrote: > > > On Thu, Apr 04, 2019 at 10:48:17PM +0100, Richard Purdie wrote: > > > > On Thu, 2019-04-04 at 18:00 +0200, Alexander Kanavin wrote: > > > > > As ptest-runner communicates with child processes via > > > > > pipe2(), > > > > > the corresponding channels are not attached to a pty. In that > > > > > situation stdio facilities like printf() or fwrite() are > > > > > fully > > > > > buffered. If a ptest would use them, without bothering > > > > > to fflush() the output, ptest-runner will only receive what > > > > > was written by the child ptest process after a buffer gets > > > > > filled. > > > > > If the unit tests are proceeding slowly, this may mean that > > > > > ptest-runner will erroneously timeout due to an apparent lack > > > > > of > > > > > 'signs of life' from the child process. > > > > > > > > > > stdbuf utility from coreutils adjusts the buffering to a > > > > > line- > > > > > buffered > > > > > one, and so ptest-runner will get the lines as soon as they > > > > > are > > > > > written. > > > > > > > > > > Signed-off-by: Alexander Kanavin > > > > > --- > > > > > utils.c | 7 ++- > > > > > 1 file changed, 2 insertions(+), 5 deletions(-) > > > > > > > > I'm a little torn on this. I noticed some of the run-ptest > > > > scripts > > > > use > > > > "| sed -u" whilst the one you were seeing problems with uses "| > > > > sed" > > > > without -u. > > > > > > > > We may want to consider strongly recommending -u. I'm testing a > > > > patch > > > > with some tweaks like that in it... > > > > > > Please no. I'm running images without sed and using busybox sed > > > instead, and that > > > doesn't support -u. I'd rather be compatible with sed from > > > busybox to > > > keep changes > > > to images minimal (e.g. install of additional packages) before > > > executing ptests. > > > > The other alternative option being proposed is for ptest-runner to > > depend on coreutils which is worse? > > GNU sed does not come from coreutils but from sed recipe. > > Your call in the end. I just provided my point of view. The original patch in this thread from Alex needs stdbuf which would add a coreutils depends to ptest-runner for everything as I understand it. > > I did test the -u option to sed in the openssh ptest runner and it > > did > > fix the problems we've been seeing. > > > > I'm open to other alternatives but the -u option to sed is looking > > like > > the best one we have right now. These bugs are making our testing > > of > > ptests effectively useless and unpredictable so this is a serious > > issue... > > Understood. I hope you could also add 'set -eux' to all ptest shell > scripts. Many of them seem to be missing shell script error handling > and failures like providing -u to busybox sed may go unnoticed. Patches would be welcome to help clean up some of these scripts. I feel like I'm fighting a lonely battle trying to get any of this to work at times :( The good news is the automated monitoring of the ptest results should catch problems like that as if a chunk of ptests "disappear", the new testing should quickly highlight that. (We should improve the scripts, we just also have another way to spot problems) Cheers, Richard -- ___ yocto mailing list yocto@yoctoproject.org https://lists.yoctoproject.org/listinfo/yocto
Re: [yocto] [ptest-runner] Run ptests via stdbuf configured to line-buffering
On Fri, Apr 05, 2019 at 07:46:00AM +0100, richard.pur...@linuxfoundation.org wrote: > On Fri, 2019-04-05 at 06:16 +, mikko.rap...@bmw.de wrote: > > On Thu, Apr 04, 2019 at 10:48:17PM +0100, Richard Purdie wrote: > > > On Thu, 2019-04-04 at 18:00 +0200, Alexander Kanavin wrote: > > > > As ptest-runner communicates with child processes via pipe2(), > > > > the corresponding channels are not attached to a pty. In that > > > > situation stdio facilities like printf() or fwrite() are fully > > > > buffered. If a ptest would use them, without bothering > > > > to fflush() the output, ptest-runner will only receive what > > > > was written by the child ptest process after a buffer gets > > > > filled. > > > > If the unit tests are proceeding slowly, this may mean that > > > > ptest-runner will erroneously timeout due to an apparent lack of > > > > 'signs of life' from the child process. > > > > > > > > stdbuf utility from coreutils adjusts the buffering to a line- > > > > buffered > > > > one, and so ptest-runner will get the lines as soon as they are > > > > written. > > > > > > > > Signed-off-by: Alexander Kanavin > > > > --- > > > > utils.c | 7 ++- > > > > 1 file changed, 2 insertions(+), 5 deletions(-) > > > > > > I'm a little torn on this. I noticed some of the run-ptest scripts > > > use > > > "| sed -u" whilst the one you were seeing problems with uses "| > > > sed" > > > without -u. > > > > > > We may want to consider strongly recommending -u. I'm testing a > > > patch > > > with some tweaks like that in it... > > > > Please no. I'm running images without sed and using busybox sed > > instead, and that > > doesn't support -u. I'd rather be compatible with sed from busybox to > > keep changes > > to images minimal (e.g. install of additional packages) before > > executing ptests. > > The other alternative option being proposed is for ptest-runner to > depend on coreutils which is worse? GNU sed does not come from coreutils but from sed recipe. Your call in the end. I just provided my point of view. > I did test the -u option to sed in the openssh ptest runner and it did > fix the problems we've been seeing. > > I'm open to other alternatives but the -u option to sed is looking like > the best one we have right now. These bugs are making our testing of > ptests effectively useless and unpredictable so this is a serious > issue... Understood. I hope you could also add 'set -eux' to all ptest shell scripts. Many of them seem to be missing shell script error handling and failures like providing -u to busybox sed may go unnoticed. -Mikko -- ___ yocto mailing list yocto@yoctoproject.org https://lists.yoctoproject.org/listinfo/yocto
Re: [yocto] [ptest-runner] Run ptests via stdbuf configured to line-buffering
On Fri, 2019-04-05 at 06:16 +, mikko.rap...@bmw.de wrote: > On Thu, Apr 04, 2019 at 10:48:17PM +0100, Richard Purdie wrote: > > On Thu, 2019-04-04 at 18:00 +0200, Alexander Kanavin wrote: > > > As ptest-runner communicates with child processes via pipe2(), > > > the corresponding channels are not attached to a pty. In that > > > situation stdio facilities like printf() or fwrite() are fully > > > buffered. If a ptest would use them, without bothering > > > to fflush() the output, ptest-runner will only receive what > > > was written by the child ptest process after a buffer gets > > > filled. > > > If the unit tests are proceeding slowly, this may mean that > > > ptest-runner will erroneously timeout due to an apparent lack of > > > 'signs of life' from the child process. > > > > > > stdbuf utility from coreutils adjusts the buffering to a line- > > > buffered > > > one, and so ptest-runner will get the lines as soon as they are > > > written. > > > > > > Signed-off-by: Alexander Kanavin > > > --- > > > utils.c | 7 ++- > > > 1 file changed, 2 insertions(+), 5 deletions(-) > > > > I'm a little torn on this. I noticed some of the run-ptest scripts > > use > > "| sed -u" whilst the one you were seeing problems with uses "| > > sed" > > without -u. > > > > We may want to consider strongly recommending -u. I'm testing a > > patch > > with some tweaks like that in it... > > Please no. I'm running images without sed and using busybox sed > instead, and that > doesn't support -u. I'd rather be compatible with sed from busybox to > keep changes > to images minimal (e.g. install of additional packages) before > executing ptests. The other alternative option being proposed is for ptest-runner to depend on coreutils which is worse? I did test the -u option to sed in the openssh ptest runner and it did fix the problems we've been seeing. I'm open to other alternatives but the -u option to sed is looking like the best one we have right now. These bugs are making our testing of ptests effectively useless and unpredictable so this is a serious issue... Cheers, Richard -- ___ yocto mailing list yocto@yoctoproject.org https://lists.yoctoproject.org/listinfo/yocto
Re: [yocto] [ptest-runner] Run ptests via stdbuf configured to line-buffering
On Thu, Apr 04, 2019 at 10:48:17PM +0100, Richard Purdie wrote: > On Thu, 2019-04-04 at 18:00 +0200, Alexander Kanavin wrote: > > As ptest-runner communicates with child processes via pipe2(), > > the corresponding channels are not attached to a pty. In that > > situation stdio facilities like printf() or fwrite() are fully > > buffered. If a ptest would use them, without bothering > > to fflush() the output, ptest-runner will only receive what > > was written by the child ptest process after a buffer gets filled. > > If the unit tests are proceeding slowly, this may mean that > > ptest-runner will erroneously timeout due to an apparent lack of > > 'signs of life' from the child process. > > > > stdbuf utility from coreutils adjusts the buffering to a line- > > buffered > > one, and so ptest-runner will get the lines as soon as they are > > written. > > > > Signed-off-by: Alexander Kanavin > > --- > > utils.c | 7 ++- > > 1 file changed, 2 insertions(+), 5 deletions(-) > > I'm a little torn on this. I noticed some of the run-ptest scripts use > "| sed -u" whilst the one you were seeing problems with uses "| sed" > without -u. > > We may want to consider strongly recommending -u. I'm testing a patch > with some tweaks like that in it... Please no. I'm running images without sed and using busybox sed instead, and that doesn't support -u. I'd rather be compatible with sed from busybox to keep changes to images minimal (e.g. install of additional packages) before executing ptests. -Mikko -- ___ yocto mailing list yocto@yoctoproject.org https://lists.yoctoproject.org/listinfo/yocto
Re: [yocto] [ptest-runner] Run ptests via stdbuf configured to line-buffering
On Thu, 2019-04-04 at 18:00 +0200, Alexander Kanavin wrote: > As ptest-runner communicates with child processes via pipe2(), > the corresponding channels are not attached to a pty. In that > situation stdio facilities like printf() or fwrite() are fully > buffered. If a ptest would use them, without bothering > to fflush() the output, ptest-runner will only receive what > was written by the child ptest process after a buffer gets filled. > If the unit tests are proceeding slowly, this may mean that > ptest-runner will erroneously timeout due to an apparent lack of > 'signs of life' from the child process. > > stdbuf utility from coreutils adjusts the buffering to a line- > buffered > one, and so ptest-runner will get the lines as soon as they are > written. > > Signed-off-by: Alexander Kanavin > --- > utils.c | 7 ++- > 1 file changed, 2 insertions(+), 5 deletions(-) I'm a little torn on this. I noticed some of the run-ptest scripts use "| sed -u" whilst the one you were seeing problems with uses "| sed" without -u. We may want to consider strongly recommending -u. I'm testing a patch with some tweaks like that in it... Cheers, Richard -- ___ yocto mailing list yocto@yoctoproject.org https://lists.yoctoproject.org/listinfo/yocto
Re: [yocto] [ptest-runner] Run ptests via stdbuf configured to line-buffering
On Thu, 2019-04-04 at 18:00 +0200, Alexander Kanavin wrote:
> As ptest-runner communicates with child processes via pipe2(),
> the corresponding channels are not attached to a pty. In that
> situation stdio facilities like printf() or fwrite() are fully
> buffered. If a ptest would use them, without bothering
> to fflush() the output, ptest-runner will only receive what
> was written by the child ptest process after a buffer gets filled.
> If the unit tests are proceeding slowly, this may mean that
> ptest-runner will erroneously timeout due to an apparent lack of
> 'signs of life' from the child process.
>
> stdbuf utility from coreutils adjusts the buffering to a line-
> buffered
> one, and so ptest-runner will get the lines as soon as they are
> written.
>
> Signed-off-by: Alexander Kanavin
> ---
> utils.c | 7 ++-
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/utils.c b/utils.c
> index 504df0b..1376e39 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -243,16 +243,13 @@ filter_ptests(struct ptest_list *head, char
> **ptests, int ptest_num)
> static inline void
> run_child(char *run_ptest, int fd_stdout, int fd_stderr)
> {
> - char **argv = malloc(sizeof(char) * 2);
> + char* argv[] = {"stdbuf", "-oL", "-eL", "./run-ptest", NULL};
Should be run_ptest the from the function argument, not the string
literal "./run-ptest" ?
> chdir(dirname(strdup(run_ptest)));
>
> - argv[0] = run_ptest;
> - argv[1] = NULL;
> -
> dup2(fd_stdout, STDOUT_FILENO);
> // XXX: Redirect stderr to stdout to avoid buffer ordering
> problems.
> dup2(fd_stdout, STDERR_FILENO);
> - execv(run_ptest, argv);
> + execvp(argv[0], argv);
>
> exit(1);
> }
> --
> 2.17.1
>
--
Joshua Watt
--
___
yocto mailing list
yocto@yoctoproject.org
https://lists.yoctoproject.org/listinfo/yocto
Re: [yocto] [ptest-runner] Run ptests via stdbuf configured to line-buffering
Bah, sorry, debugging got into the patch.
Will resend.
Alex
On Thu, 4 Apr 2019 at 17:34, Alexander Kanavin wrote:
>
> As ptest-runner communicates with child processes via pipe2(),
> the corresponding channels are not attached to a pty. In that
> situation stdio facilities like printf() or fwrite() are fully
> buffered. If a ptest would use them, without bothering
> to fflush() the output, ptest-runner will only receive what
> was written by the child ptest process after a buffer gets filled.
> If the unit tests are proceeding slowly, this may mean that
> ptest-runner will erroneously timeout due to an apparent lack of
> 'signs of life' from the child process.
>
> stdbuf utility from coreutils adjusts the buffering to a line-buffered
> one, and so ptest-runner will get the lines as soon as they are
> written.
>
> Signed-off-by: Alexander Kanavin
> ---
> utils.c | 11 ---
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/utils.c b/utils.c
> index 504df0b..c130fbd 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -243,16 +243,13 @@ filter_ptests(struct ptest_list *head, char **ptests,
> int ptest_num)
> static inline void
> run_child(char *run_ptest, int fd_stdout, int fd_stderr)
> {
> - char **argv = malloc(sizeof(char) * 2);
> + char* argv[] = {"stdbuf", "-oL", "-eL", "./run-ptest", NULL};
> chdir(dirname(strdup(run_ptest)));
>
> - argv[0] = run_ptest;
> - argv[1] = NULL;
> -
> dup2(fd_stdout, STDOUT_FILENO);
> // XXX: Redirect stderr to stdout to avoid buffer ordering problems.
> dup2(fd_stdout, STDERR_FILENO);
> - execv(run_ptest, argv);
> + execvp(argv[0], argv);
>
> exit(1);
> }
> @@ -291,12 +288,12 @@ wait_child(const char *ptest_dir, const char
> *run_ptest, pid_t pid,
>
> if (pfds[0].revents != 0) {
> while ((n = read(fds[0], buf,
> WAIT_CHILD_BUF_MAX_SIZE)) > 0)
> - fwrite(buf, n, 1, fps[0]);
> + fwrite(buf, n, 1, fps[0]);
> fwrite("hallo", 5 ,1 ,fps[0]); fflush(fps[0]);
> }
>
> if (pfds[1].revents != 0) {
> while ((n = read(fds[1], buf,
> WAIT_CHILD_BUF_MAX_SIZE)) > 0)
> - fwrite(buf, n, 1, fps[1]);
> + fwrite(buf, n, 1, fps[1]);
> fwrite("hallo-err", 9 ,1 ,fps[0]); fflush(fps[1]);
> }
>
> clock_gettime(clock, );
> --
> 2.17.1
>
--
___
yocto mailing list
yocto@yoctoproject.org
https://lists.yoctoproject.org/listinfo/yocto
