* [LTP] [PATCH v3 5/5] busy_poll: Move TST_{SETUP, CLEANUP} to the tests
@ 2022-04-28 6:56 Petr Vorel
2022-04-29 16:05 ` Martin Doucha
0 siblings, 1 reply; 6+ messages in thread
From: Petr Vorel @ 2022-04-28 6:56 UTC (permalink / raw)
To: ltp; +Cc: Martin Doucha
Although having variables in both busy_poll_lib.sh and the tests which
are using it isn't optimal, hooking up callbacks on the reverse end of
include is even worse code.
Suggested-by: Martin Doucha <mdoucha@suse.cz>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Follow-up of "v3 shell: Cleanup getopts usage" patchset [1],
replacing first commit.
Kind regards,
Petr
[1] https://patchwork.ozlabs.org/project/ltp/list/?series=297175
testcases/network/busy_poll/busy_poll01.sh | 3 +++
testcases/network/busy_poll/busy_poll02.sh | 3 +++
testcases/network/busy_poll/busy_poll03.sh | 2 ++
testcases/network/busy_poll/busy_poll_lib.sh | 3 +--
4 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/testcases/network/busy_poll/busy_poll01.sh b/testcases/network/busy_poll/busy_poll01.sh
index 65f4db3fe..1f7097771 100755
--- a/testcases/network/busy_poll/busy_poll01.sh
+++ b/testcases/network/busy_poll/busy_poll01.sh
@@ -4,6 +4,9 @@
#
# Author: Alexey Kodanev <alexey.kodanev@oracle.com>
+TST_SETUP="setup"
+TST_CLEANUP="cleanup"
+
cleanup()
{
[ -n "$busy_read_old" ] && \
diff --git a/testcases/network/busy_poll/busy_poll02.sh b/testcases/network/busy_poll/busy_poll02.sh
index ebae4d2f5..634bbd6bd 100755
--- a/testcases/network/busy_poll/busy_poll02.sh
+++ b/testcases/network/busy_poll/busy_poll02.sh
@@ -4,6 +4,9 @@
#
# Author: Alexey Kodanev <alexey.kodanev@oracle.com>
+TST_SETUP="setup"
+TST_CLEANUP="cleanup"
+
cleanup()
{
[ -n "$busy_poll_old" ] && \
diff --git a/testcases/network/busy_poll/busy_poll03.sh b/testcases/network/busy_poll/busy_poll03.sh
index 04d5978f7..b2e1c0a7a 100755
--- a/testcases/network/busy_poll/busy_poll03.sh
+++ b/testcases/network/busy_poll/busy_poll03.sh
@@ -4,6 +4,8 @@
#
# Author: Alexey Kodanev <alexey.kodanev@oracle.com>
+TST_SETUP="setup"
+TST_CLEANUP="cleanup"
TST_TEST_DATA="udp udp_lite"
cleanup()
diff --git a/testcases/network/busy_poll/busy_poll_lib.sh b/testcases/network/busy_poll/busy_poll_lib.sh
index de61d3fcd..446ae3d65 100755
--- a/testcases/network/busy_poll/busy_poll_lib.sh
+++ b/testcases/network/busy_poll/busy_poll_lib.sh
@@ -1,10 +1,9 @@
#!/bin/sh
# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) Linux Test Project, 2016-2022
# Copyright (c) 2016-2018 Oracle and/or its affiliates. All Rights Reserved.
-TST_SETUP="setup"
TST_TESTFUNC="test"
-TST_CLEANUP="cleanup"
TST_MIN_KVER="3.11"
TST_NEEDS_TMPDIR=1
TST_NEEDS_ROOT=1
--
2.35.3
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [LTP] [PATCH v3 5/5] busy_poll: Move TST_{SETUP, CLEANUP} to the tests
2022-04-28 6:56 [LTP] [PATCH v3 5/5] busy_poll: Move TST_{SETUP, CLEANUP} to the tests Petr Vorel
@ 2022-04-29 16:05 ` Martin Doucha
2022-04-29 18:09 ` Petr Vorel
0 siblings, 1 reply; 6+ messages in thread
From: Martin Doucha @ 2022-04-29 16:05 UTC (permalink / raw)
To: Petr Vorel, ltp
Hi,
TST_TESTFUNC should be moved to individual test scripts as well, for the
same reason. Other than that, the whole patchset looks good:
Reviewed-by: Martin Doucha <mdoucha@suse.cz>
On 28. 04. 22 8:56, Petr Vorel wrote:
> Although having variables in both busy_poll_lib.sh and the tests which
> are using it isn't optimal, hooking up callbacks on the reverse end of
> include is even worse code.
>
> Suggested-by: Martin Doucha <mdoucha@suse.cz>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> Follow-up of "v3 shell: Cleanup getopts usage" patchset [1],
> replacing first commit.
>
> Kind regards,
> Petr
>
> [1] https://patchwork.ozlabs.org/project/ltp/list/?series=297175
>
> testcases/network/busy_poll/busy_poll01.sh | 3 +++
> testcases/network/busy_poll/busy_poll02.sh | 3 +++
> testcases/network/busy_poll/busy_poll03.sh | 2 ++
> testcases/network/busy_poll/busy_poll_lib.sh | 3 +--
> 4 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/testcases/network/busy_poll/busy_poll01.sh b/testcases/network/busy_poll/busy_poll01.sh
> index 65f4db3fe..1f7097771 100755
> --- a/testcases/network/busy_poll/busy_poll01.sh
> +++ b/testcases/network/busy_poll/busy_poll01.sh
> @@ -4,6 +4,9 @@
> #
> # Author: Alexey Kodanev <alexey.kodanev@oracle.com>
>
> +TST_SETUP="setup"
> +TST_CLEANUP="cleanup"
> +
> cleanup()
> {
> [ -n "$busy_read_old" ] && \
> diff --git a/testcases/network/busy_poll/busy_poll02.sh b/testcases/network/busy_poll/busy_poll02.sh
> index ebae4d2f5..634bbd6bd 100755
> --- a/testcases/network/busy_poll/busy_poll02.sh
> +++ b/testcases/network/busy_poll/busy_poll02.sh
> @@ -4,6 +4,9 @@
> #
> # Author: Alexey Kodanev <alexey.kodanev@oracle.com>
>
> +TST_SETUP="setup"
> +TST_CLEANUP="cleanup"
> +
> cleanup()
> {
> [ -n "$busy_poll_old" ] && \
> diff --git a/testcases/network/busy_poll/busy_poll03.sh b/testcases/network/busy_poll/busy_poll03.sh
> index 04d5978f7..b2e1c0a7a 100755
> --- a/testcases/network/busy_poll/busy_poll03.sh
> +++ b/testcases/network/busy_poll/busy_poll03.sh
> @@ -4,6 +4,8 @@
> #
> # Author: Alexey Kodanev <alexey.kodanev@oracle.com>
>
> +TST_SETUP="setup"
> +TST_CLEANUP="cleanup"
> TST_TEST_DATA="udp udp_lite"
>
> cleanup()
> diff --git a/testcases/network/busy_poll/busy_poll_lib.sh b/testcases/network/busy_poll/busy_poll_lib.sh
> index de61d3fcd..446ae3d65 100755
> --- a/testcases/network/busy_poll/busy_poll_lib.sh
> +++ b/testcases/network/busy_poll/busy_poll_lib.sh
> @@ -1,10 +1,9 @@
> #!/bin/sh
> # SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) Linux Test Project, 2016-2022
> # Copyright (c) 2016-2018 Oracle and/or its affiliates. All Rights Reserved.
>
> -TST_SETUP="setup"
> TST_TESTFUNC="test"
^^ here
> -TST_CLEANUP="cleanup"
> TST_MIN_KVER="3.11"
> TST_NEEDS_TMPDIR=1
> TST_NEEDS_ROOT=1
--
Martin Doucha mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [LTP] [PATCH v3 5/5] busy_poll: Move TST_{SETUP, CLEANUP} to the tests
2022-04-29 16:05 ` Martin Doucha
@ 2022-04-29 18:09 ` Petr Vorel
2022-05-02 8:54 ` Martin Doucha
0 siblings, 1 reply; 6+ messages in thread
From: Petr Vorel @ 2022-04-29 18:09 UTC (permalink / raw)
To: Martin Doucha; +Cc: ltp
Hi Martin,
> Hi,
> TST_TESTFUNC should be moved to individual test scripts as well, for the
> same reason. Other than that, the whole patchset looks good:
I was thinking about it as well. Makes sense, but then it hides the fact that
busy_poll_lib.sh contain other API variables (missing TST_TESTFUNC suggest that
there are some variables in busy_poll_lib.sh). Anyway, I'm ok to move it there
as well, just document that sharing variables between library and test will be
always a bit problematic.
> Reviewed-by: Martin Doucha <mdoucha@suse.cz>
Thanks!
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [LTP] [PATCH v3 5/5] busy_poll: Move TST_{SETUP, CLEANUP} to the tests
2022-04-29 18:09 ` Petr Vorel
@ 2022-05-02 8:54 ` Martin Doucha
2022-05-02 13:50 ` Petr Vorel
0 siblings, 1 reply; 6+ messages in thread
From: Martin Doucha @ 2022-05-02 8:54 UTC (permalink / raw)
To: Petr Vorel; +Cc: ltp
On 29. 04. 22 20:09, Petr Vorel wrote:
> I was thinking about it as well. Makes sense, but then it hides the fact that
> busy_poll_lib.sh contain other API variables (missing TST_TESTFUNC suggest that
> there are some variables in busy_poll_lib.sh). Anyway, I'm ok to move it there
> as well, just document that sharing variables between library and test will be
> always a bit problematic.
Now that you mention the other variables in busy_poll_lib.sh, I guess
that TST_MIN_KVER and TST_NEEDS_CMDS should use conditional expansion so
that the values can be changed in the tests.
--
Martin Doucha mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [LTP] [PATCH v3 5/5] busy_poll: Move TST_{SETUP, CLEANUP} to the tests
2022-05-02 8:54 ` Martin Doucha
@ 2022-05-02 13:50 ` Petr Vorel
2022-05-02 14:00 ` Martin Doucha
0 siblings, 1 reply; 6+ messages in thread
From: Petr Vorel @ 2022-05-02 13:50 UTC (permalink / raw)
To: Martin Doucha; +Cc: ltp
Hi Martin,
> On 29. 04. 22 20:09, Petr Vorel wrote:
> > I was thinking about it as well. Makes sense, but then it hides the fact that
> > busy_poll_lib.sh contain other API variables (missing TST_TESTFUNC suggest that
> > there are some variables in busy_poll_lib.sh). Anyway, I'm ok to move it there
> > as well, just document that sharing variables between library and test will be
> > always a bit problematic.
> Now that you mention the other variables in busy_poll_lib.sh, I guess
> that TST_MIN_KVER and TST_NEEDS_CMDS should use conditional expansion so
> that the values can be changed in the tests.
Makes sense. But I'd probably prefer to postpone this commit,
because TST_NEEDS_CMDS will be in more shell libraries.
Because even this cleanup so far wasn't my intention, I'd prefer to get it
merged soon so that I can post rebased fixes for tst_net.sh with disabled IPv6
(to get them merged before release).
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [LTP] [PATCH v3 5/5] busy_poll: Move TST_{SETUP, CLEANUP} to the tests
2022-05-02 13:50 ` Petr Vorel
@ 2022-05-02 14:00 ` Martin Doucha
0 siblings, 0 replies; 6+ messages in thread
From: Martin Doucha @ 2022-05-02 14:00 UTC (permalink / raw)
To: Petr Vorel; +Cc: ltp
On 02. 05. 22 15:50, Petr Vorel wrote:
> Makes sense. But I'd probably prefer to postpone this commit,
> because TST_NEEDS_CMDS will be in more shell libraries.
> Because even this cleanup so far wasn't my intention, I'd prefer to get it
> merged soon so that I can post rebased fixes for tst_net.sh with disabled IPv6
> (to get them merged before release).
Sure, if you plan to fix that in multiple shell libraries, then a
separate patchset is a good idea.
--
Martin Doucha mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-05-02 14:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-04-28 6:56 [LTP] [PATCH v3 5/5] busy_poll: Move TST_{SETUP, CLEANUP} to the tests Petr Vorel
2022-04-29 16:05 ` Martin Doucha
2022-04-29 18:09 ` Petr Vorel
2022-05-02 8:54 ` Martin Doucha
2022-05-02 13:50 ` Petr Vorel
2022-05-02 14:00 ` Martin Doucha
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.