Re: [ovs-dev] [PATCH v3] tests: Simplify and improve the daemon tests.
On Mon, 10 Dec 2018 09:43:19 -0800
Ben Pfaff wrote:
> The daemon tests used files a lot when shell variables were easier to use
> and easier to understand. This commit changes that.
>
> The tests created empty databases that aren't really needed anymore. This
> commit changes them to use the ovsdb-server --no-db option instead.
>
> The tests had a lot of common code for checking the ancestry of processes.
> This commit factors out a new shell function check_ancestors.
>
> The tests tended to use random pidfile names. This switches to just using
> the defaults, which are fine.
>
> The tests didn't check the names of the child processes. This adds those
> checks using the new check_process_name shell function. This should avoid
> regression of the bug fixed by commit 266f79e32c60 ("daemon-unix: Use
> same name for original or restarted children.")
>
> Other minor improvements too.
>
> I only made small updates to the Windows-specific test, because it is hard
> for me to verify.
>
> Acked-by: Alin Gabriel Serdean
> Signed-off-by: Ben Pfaff
> ---
> v1->v2: Rebased, added ack.
> v2->v3: Fix some mixups in v2 from muddling two patches together.
>
> tests/daemon-py.at | 218
> +
> tests/daemon.at| 212 ---
> 2 files changed, 220 insertions(+), 210 deletions(-)
>
[...]
> diff --git a/tests/daemon.at b/tests/daemon.at
> index fa4844ae8eaa..6382d856bd95 100644
> --- a/tests/daemon.at
> +++ b/tests/daemon.at
> @@ -1,5 +1,48 @@
> AT_BANNER([daemon unit tests - C])
>
> +OVS_START_SHELL_HELPERS
> +# check_process_name PID NAME
> +#
> +# On Linux, make sure that the name of process PID is NAME.
> +# (On other systems, don't bother.)
> +if test -e /proc/$$/comm; then
> +check_process_name() {
> +AT_CHECK_UNQUOTED([cat /proc/$1/comm], [0], [$2
> +])
Hi,
this function doesn't work when you build OVS with --enable-shared
since, in this case, ovsdb-server is launched by libtool and so
cat /proc/$1/comm is lt-ovsdb-server:
## --- ##
## openvswitch 2.10.90 test suite. ##
## --- ##
136. daemon.at:76: testing daemon --monitor ...
./daemon.at:84: ovsdb-server --monitor --pidfile --no-db 2>/dev/null & echo $!
stdout:
22575
daemon.at:87: waiting until test -s ovsdb-server.pid...
daemon.at:87: wait succeeded after 1 seconds
checking ancestry: 22599 22575
./daemon.at:23: kill -0 $child
./daemon.at:26: parent_pid $child
stdout:
22575
./daemon.at:10: cat /proc/$1/comm
--- - 2018-12-16 13:08:59.288838410 +0100
+++ /home/tredaell/dev/sources/ovs/tests/testsuite.dir/at-groups/136/stdout
2018-12-16 13:08:59.284995408 +0100
@@ -1,2 +1,2 @@
-ovsdb-server
+lt-ovsdb-server
136. daemon.at:76: FAILED (daemon.at:10)
Detected by my daily Fedora/CentOS CI:
https://gitlab.com/drizzt/ovs/pipelines/40295102
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] tests: Simplify and improve the daemon tests.
On Sun, Dec 16, 2018 at 01:11:15PM +0100, Timothy Redaelli wrote:
> On Mon, 10 Dec 2018 09:43:19 -0800
> Ben Pfaff wrote:
>
> > The daemon tests used files a lot when shell variables were easier to use
> > and easier to understand. This commit changes that.
> >
> > The tests created empty databases that aren't really needed anymore. This
> > commit changes them to use the ovsdb-server --no-db option instead.
> >
> > The tests had a lot of common code for checking the ancestry of processes.
> > This commit factors out a new shell function check_ancestors.
> >
> > The tests tended to use random pidfile names. This switches to just using
> > the defaults, which are fine.
> >
> > The tests didn't check the names of the child processes. This adds those
> > checks using the new check_process_name shell function. This should avoid
> > regression of the bug fixed by commit 266f79e32c60 ("daemon-unix: Use
> > same name for original or restarted children.")
> >
> > Other minor improvements too.
> >
> > I only made small updates to the Windows-specific test, because it is hard
> > for me to verify.
> >
> > Acked-by: Alin Gabriel Serdean
> > Signed-off-by: Ben Pfaff
> > ---
> > v1->v2: Rebased, added ack.
> > v2->v3: Fix some mixups in v2 from muddling two patches together.
> >
> > tests/daemon-py.at | 218
> > +
> > tests/daemon.at| 212
> > ---
> > 2 files changed, 220 insertions(+), 210 deletions(-)
> >
> [...]
> > diff --git a/tests/daemon.at b/tests/daemon.at
> > index fa4844ae8eaa..6382d856bd95 100644
> > --- a/tests/daemon.at
> > +++ b/tests/daemon.at
> > @@ -1,5 +1,48 @@
> > AT_BANNER([daemon unit tests - C])
> >
> > +OVS_START_SHELL_HELPERS
> > +# check_process_name PID NAME
> > +#
> > +# On Linux, make sure that the name of process PID is NAME.
> > +# (On other systems, don't bother.)
> > +if test -e /proc/$$/comm; then
> > +check_process_name() {
> > +AT_CHECK_UNQUOTED([cat /proc/$1/comm], [0], [$2
> > +])
>
> Hi,
> this function doesn't work when you build OVS with --enable-shared
> since, in this case, ovsdb-server is launched by libtool and so
> cat /proc/$1/comm is lt-ovsdb-server:
Thanks for the report. I don't normally build with --enable-shared, so
that's why I missed it.
I rebuilt with --enable-shared and, oddly, couldn't reproduce the
problem, but the fix should be easy and I posted it:
https://patchwork.ozlabs.org/patch/1014162/
Does it fix the problem you see?
Thanks,
Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] tests: Simplify and improve the daemon tests.
> On 10 Dec 2018, at 19:43, Ben Pfaff wrote:
>
> The daemon tests used files a lot when shell variables were easier to use
> and easier to understand. This commit changes that.
>
> The tests created empty databases that aren't really needed anymore. This
> commit changes them to use the ovsdb-server --no-db option instead.
>
> The tests had a lot of common code for checking the ancestry of processes.
> This commit factors out a new shell function check_ancestors.
>
> The tests tended to use random pidfile names. This switches to just using
> the defaults, which are fine.
>
> The tests didn't check the names of the child processes. This adds those
> checks using the new check_process_name shell function. This should avoid
> regression of the bug fixed by commit 266f79e32c60 ("daemon-unix: Use
> same name for original or restarted children.")
>
> Other minor improvements too.
>
> I only made small updates to the Windows-specific test, because it is hard
> for me to verify.
>
> Acked-by: Alin Gabriel Serdean
> Signed-off-by: Ben Pfaff
> ---
> v1->v2: Rebased, added ack.
> v2->v3: Fix some mixups in v2 from muddling two patches together.
>
> tests/daemon-py.at | 218 +
> tests/daemon.at| 212 ---
> 2 files changed, 220 insertions(+), 210 deletions(-)
>
I did another run of testing on Windows and everything was fine.
Acked-by: Alin Gabriel Serdean
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
