* [LTP] new shell library
@ 2016-10-03 14:58 Cyril Hrubis
2016-10-04 8:23 ` Jan Stancek
0 siblings, 1 reply; 25+ messages in thread
From: Cyril Hrubis @ 2016-10-03 14:58 UTC (permalink / raw)
To: ltp
Hi!
I've played bit with the shell test library in order to make it more
aligned with the C library, have a look at:
https://github.com/metan-ucw/ltp
So far I've converted all testcases in commands/ directory, you can have
a look at last 12 commits in the repo to see how the new shell library
and testcases looks like. I would love to get someone to review the code
before I attempt to convert more testcases.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] new shell library
2016-10-03 14:58 [LTP] new shell library Cyril Hrubis
@ 2016-10-04 8:23 ` Jan Stancek
2016-10-04 8:45 ` Cyril Hrubis
0 siblings, 1 reply; 25+ messages in thread
From: Jan Stancek @ 2016-10-04 8:23 UTC (permalink / raw)
To: ltp
----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: ltp@lists.linux.it
> Cc: "Jan Stancek" <jstancek@redhat.com>
> Sent: Monday, 3 October, 2016 4:58:35 PM
> Subject: new shell library
>
> Hi!
> I've played bit with the shell test library in order to make it more
> aligned with the C library, have a look at:
>
> https://github.com/metan-ucw/ltp
>
> So far I've converted all testcases in commands/ directory, you can have
> a look at last 12 commits in the repo to see how the new shell library
> and testcases looks like. I would love to get someone to review the code
> before I attempt to convert more testcases.
Hi,
As a person who sees this first time, my immediate concern was,
how do I know what variables functions names are reserved
(without need to read entire tst_test.sh) and what happens if
I make a typo?
It also forces me to use specific names as the names
of my functions (e.g. "test", which presumably shadows test(1)).
And there's now requirement on when you can include new library,
it has to be at the end of file, because there are side-effects
(it starts the test on inclusion).
So, I was thinking:
. tst_test.sh
...
tst_def TID="du01"
tst_def TCNT=23
tst_def TST_SETUP mysetup
tst_def TST_CLEANUP mycleanup
tst_def TST_TESTFUNC mytest
tst_def TST_NEEDS_TMPDIR=1
tst_def TST_NEEDS_CMDS="dd du stat"
tst_start
tst_def - makes it clear that this variable defines something for tst_test,
checks for potential typos, doesn't allow non-existent ones,
is a single place in tst_test.sh where I can see all possible variables together
TST_TESTFUNC - allows me to give test function any name
tst_start - starts the test
allows us to keep all includes at one place
IMO all of these would make the interface between tst_test.sh and actual
test more apparent and it's still in the spirit of C counter-part.
Regards,
Jan
>
> --
> Cyril Hrubis
> chrubis@suse.cz
>
^ permalink raw reply [flat|nested] 25+ messages in thread* [LTP] new shell library
2016-10-04 8:23 ` Jan Stancek
@ 2016-10-04 8:45 ` Cyril Hrubis
2016-10-04 9:02 ` Jan Stancek
0 siblings, 1 reply; 25+ messages in thread
From: Cyril Hrubis @ 2016-10-04 8:45 UTC (permalink / raw)
To: ltp
Hi!
> As a person who sees this first time, my immediate concern was,
> how do I know what variables functions names are reserved
> (without need to read entire tst_test.sh) and what happens if
> I make a typo?
>
> It also forces me to use specific names as the names
> of my functions (e.g. "test", which presumably shadows test(1)).
That is a valid concern. I guess that passing everyting
(setup/cleanup/test/etc.) via TST_FOO variables would be much better.
I was trying to reduce the number of lines per test as much as possible
but this is perhaps too agressive and the end result is complicated
automagic that is hard to understand.
> And there's now requirement on when you can include new library,
> it has to be at the end of file, because there are side-effects
> (it starts the test on inclusion).
>
> So, I was thinking:
>
> . tst_test.sh
>
> ...
>
> tst_def TID="du01"
> tst_def TCNT=23
> tst_def TST_SETUP mysetup
> tst_def TST_CLEANUP mycleanup
> tst_def TST_TESTFUNC mytest
> tst_def TST_NEEDS_TMPDIR=1
> tst_def TST_NEEDS_CMDS="dd du stat"
> tst_start
>
> tst_def - makes it clear that this variable defines something for tst_test,
> checks for potential typos, doesn't allow non-existent ones,
> is a single place in tst_test.sh where I can see all possible variables together
Hmm, I do not like this indirection too much. This obscures what is
happening in similar manner to my automagic.
What about we do it as:
TST_ID="du01"
TST_CNT=23
TST_SETUP=setup
TST_CLEANUP=cleanup
TST_TESTFUNC=du_test
TST_NEEDS_TMPDIR=1
TST_NEEDS_CMDS="dd du stat"
. tst_test.sh
# the actuall test code
tst_run
And if you are worried about typos in the TST_ variables we can write a
really simple script that checks for these, something in the spirit of
checkpatch.pl for shell code (bonus is that we can also teach it to look
for common mistakes in shell code as well).
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] new shell library
2016-10-04 8:45 ` Cyril Hrubis
@ 2016-10-04 9:02 ` Jan Stancek
2016-10-04 9:35 ` Cyril Hrubis
0 siblings, 1 reply; 25+ messages in thread
From: Jan Stancek @ 2016-10-04 9:02 UTC (permalink / raw)
To: ltp
----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Tuesday, 4 October, 2016 10:45:32 AM
> Subject: Re: new shell library
>
> Hi!
> > As a person who sees this first time, my immediate concern was,
> > how do I know what variables functions names are reserved
> > (without need to read entire tst_test.sh) and what happens if
> > I make a typo?
> >
> > It also forces me to use specific names as the names
> > of my functions (e.g. "test", which presumably shadows test(1)).
>
> That is a valid concern. I guess that passing everyting
> (setup/cleanup/test/etc.) via TST_FOO variables would be much better.
>
> I was trying to reduce the number of lines per test as much as possible
> but this is perhaps too agressive and the end result is complicated
> automagic that is hard to understand.
>
> > And there's now requirement on when you can include new library,
> > it has to be at the end of file, because there are side-effects
> > (it starts the test on inclusion).
> >
> > So, I was thinking:
> >
> > . tst_test.sh
> >
> > ...
> >
> > tst_def TID="du01"
> > tst_def TCNT=23
> > tst_def TST_SETUP mysetup
> > tst_def TST_CLEANUP mycleanup
> > tst_def TST_TESTFUNC mytest
> > tst_def TST_NEEDS_TMPDIR=1
> > tst_def TST_NEEDS_CMDS="dd du stat"
> > tst_start
> >
> > tst_def - makes it clear that this variable defines something for tst_test,
> > checks for potential typos, doesn't allow non-existent ones,
> > is a single place in tst_test.sh where I can see all possible
> > variables together
>
> Hmm, I do not like this indirection too much. This obscures what is
> happening in similar manner to my automagic.
>
> What about we do it as:
>
> TST_ID="du01"
> TST_CNT=23
> TST_SETUP=setup
> TST_CLEANUP=cleanup
> TST_TESTFUNC=du_test
> TST_NEEDS_TMPDIR=1
> TST_NEEDS_CMDS="dd du stat"
> . tst_test.sh
>
> # the actuall test code
>
> tst_run
OK, giving all uniform names (TST_) and adding setup/cleanup/testfunc
makes it more clear.
What role does ". tst_test.sh" have in example above? Does it
have to come after you define all variables? If so, what does
it do?
If we have tst_run, then it seems that could do all necessary
checks and setup and we could include tst_test.sh at any point.
Regards,
Jan
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] new shell library
2016-10-04 9:02 ` Jan Stancek
@ 2016-10-04 9:35 ` Cyril Hrubis
2016-10-04 9:54 ` Jan Stancek
0 siblings, 1 reply; 25+ messages in thread
From: Cyril Hrubis @ 2016-10-04 9:35 UTC (permalink / raw)
To: ltp
Hi!
> > What about we do it as:
> >
> > TST_ID="du01"
> > TST_CNT=23
> > TST_SETUP=setup
> > TST_CLEANUP=cleanup
> > TST_TESTFUNC=du_test
> > TST_NEEDS_TMPDIR=1
> > TST_NEEDS_CMDS="dd du stat"
> > . tst_test.sh
> >
> > # the actuall test code
> >
> > tst_run
>
> OK, giving all uniform names (TST_) and adding setup/cleanup/testfunc
> makes it more clear.
>
> What role does ". tst_test.sh" have in example above? Does it
> have to come after you define all variables? If so, what does
> it do?
>
> If we have tst_run, then it seems that could do all necessary
> checks and setup and we could include tst_test.sh at any point.
I somehow find it more clear to define all the variables that the
library script uses before we source it. IMHO that is better than having
them scattered all around the script, at this point both the old test.sh
and tst_test.sh scripts enforce that by doing sanity checks on the
variables when the library script is sourced.
Are any advantages for doing the sanity checks in the tst_run function?
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] new shell library
2016-10-04 9:35 ` Cyril Hrubis
@ 2016-10-04 9:54 ` Jan Stancek
2016-10-04 11:50 ` Cyril Hrubis
2016-10-12 10:08 ` Cyril Hrubis
0 siblings, 2 replies; 25+ messages in thread
From: Jan Stancek @ 2016-10-04 9:54 UTC (permalink / raw)
To: ltp
----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Tuesday, 4 October, 2016 11:35:49 AM
> Subject: Re: new shell library
>
> Hi!
> > > What about we do it as:
> > >
> > > TST_ID="du01"
> > > TST_CNT=23
> > > TST_SETUP=setup
> > > TST_CLEANUP=cleanup
> > > TST_TESTFUNC=du_test
> > > TST_NEEDS_TMPDIR=1
> > > TST_NEEDS_CMDS="dd du stat"
> > > . tst_test.sh
> > >
> > > # the actuall test code
> > >
> > > tst_run
> >
> > OK, giving all uniform names (TST_) and adding setup/cleanup/testfunc
> > makes it more clear.
> >
> > What role does ". tst_test.sh" have in example above? Does it
> > have to come after you define all variables? If so, what does
> > it do?
> >
> > If we have tst_run, then it seems that could do all necessary
> > checks and setup and we could include tst_test.sh at any point.
>
> I somehow find it more clear to define all the variables that the
> library script uses before we source it. IMHO that is better than having
> them scattered all around the script, at this point both the old test.sh
> and tst_test.sh scripts enforce that by doing sanity checks on the
> variables when the library script is sourced.
>
> Are any advantages for doing the sanity checks in the tst_run function?
Can't think of any. You (and old test.sh) convinced me to go with your
example. Let's give it couple days, if anyone else wants to weigh in.
Regards,
Jan
>
> --
> Cyril Hrubis
> chrubis@suse.cz
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] new shell library
2016-10-04 9:54 ` Jan Stancek
@ 2016-10-04 11:50 ` Cyril Hrubis
2016-10-12 10:08 ` Cyril Hrubis
1 sibling, 0 replies; 25+ messages in thread
From: Cyril Hrubis @ 2016-10-04 11:50 UTC (permalink / raw)
To: ltp
Hi!
> > I somehow find it more clear to define all the variables that the
> > library script uses before we source it. IMHO that is better than having
> > them scattered all around the script, at this point both the old test.sh
> > and tst_test.sh scripts enforce that by doing sanity checks on the
> > variables when the library script is sourced.
> >
> > Are any advantages for doing the sanity checks in the tst_run function?
>
> Can't think of any. You (and old test.sh) convinced me to go with your
> example. Let's give it couple days, if anyone else wants to weigh in.
FYI I've changed the code at [1] accordingly to our discussion as it was
easy enough, mostly just shuffling code around.
[1] https://github.com/metan-ucw/ltp/commits/master
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] new shell library
2016-10-04 9:54 ` Jan Stancek
2016-10-04 11:50 ` Cyril Hrubis
@ 2016-10-12 10:08 ` Cyril Hrubis
2016-10-12 10:32 ` Jan Stancek
1 sibling, 1 reply; 25+ messages in thread
From: Cyril Hrubis @ 2016-10-12 10:08 UTC (permalink / raw)
To: ltp
Hi!
> Can't think of any. You (and old test.sh) convinced me to go with your
> example. Let's give it couple days, if anyone else wants to weigh in.
I've started to document the new shell library (the changes to
test-writing-guidelines are now included in [1]) and also tried to think
of some library hardening, i.e. making sure to avoid common mistakes,
the only thing I came up with is [2]. What do you think about this
approach?
[1]
https://github.com/metan-ucw/ltp/commit/fe94afe17bcdd05213d96c0cf014b1a3deec9659
[2]
https://github.com/metan-ucw/ltp/commit/7ca6be805c9fa4083b7e4de7f28162fca6a4a090
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] new shell library
2016-10-12 10:08 ` Cyril Hrubis
@ 2016-10-12 10:32 ` Jan Stancek
2016-10-12 12:36 ` Cyril Hrubis
0 siblings, 1 reply; 25+ messages in thread
From: Jan Stancek @ 2016-10-12 10:32 UTC (permalink / raw)
To: ltp
----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Wednesday, 12 October, 2016 12:08:06 PM
> Subject: Re: new shell library
>
> Hi!
> > Can't think of any. You (and old test.sh) convinced me to go with your
> > example. Let's give it couple days, if anyone else wants to weigh in.
>
> I've started to document the new shell library (the changes to
> test-writing-guidelines are now included in [1]) and also tried to think
> of some library hardening, i.e. making sure to avoid common mistakes,
> the only thing I came up with is [2]. What do you think about this
> approach?
I don't have better idea than some checks on inclusion of test.sh or in tst_run.
As for things we can check, I was thinking matching "set | grep ^TST_"
against list of all supported variables. A typo or non-existing/deprecated
variable would immediately show up.
Regards,
Jan
>
> [1]
> https://github.com/metan-ucw/ltp/commit/fe94afe17bcdd05213d96c0cf014b1a3deec9659
>
> [2]
> https://github.com/metan-ucw/ltp/commit/7ca6be805c9fa4083b7e4de7f28162fca6a4a090
>
> --
> Cyril Hrubis
> chrubis@suse.cz
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] new shell library
2016-10-12 10:32 ` Jan Stancek
@ 2016-10-12 12:36 ` Cyril Hrubis
2016-10-12 13:17 ` Cyril Hrubis
0 siblings, 1 reply; 25+ messages in thread
From: Cyril Hrubis @ 2016-10-12 12:36 UTC (permalink / raw)
To: ltp
Hi!
> > I've started to document the new shell library (the changes to
> > test-writing-guidelines are now included in [1]) and also tried to think
> > of some library hardening, i.e. making sure to avoid common mistakes,
> > the only thing I came up with is [2]. What do you think about this
> > approach?
>
> I don't have better idea than some checks on inclusion of test.sh or in tst_run.
>
> As for things we can check, I was thinking matching "set | grep ^TST_"
> against list of all supported variables. A typo or non-existing/deprecated
> variable would immediately show up.
What about greping the test source as in [2] instead? Because that way
we can print error if the test source touches any of the internally used
variables as well. For instance if it tries to do anything with
TST_PASS/TST_FAIL/...
> > [2]
> > https://github.com/metan-ucw/ltp/commit/7ca6be805c9fa4083b7e4de7f28162fca6a4a090
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] new shell library
2016-10-12 12:36 ` Cyril Hrubis
@ 2016-10-12 13:17 ` Cyril Hrubis
2016-10-12 14:54 ` Jan Stancek
0 siblings, 1 reply; 25+ messages in thread
From: Cyril Hrubis @ 2016-10-12 13:17 UTC (permalink / raw)
To: ltp
Hi!
> What about greping the test source as in [2] instead? Because that way
> we can print error if the test source touches any of the internally used
> variables as well. For instance if it tries to do anything with
> TST_PASS/TST_FAIL/...
What about this one:
https://github.com/metan-ucw/ltp/commit/445e3ae253bdd11f18ec12ccc74fe99eb582eeb6
$ cat d.sh
#!/bin/sh
TST_ID="test"
TST_TESTFUNC=do_test
. tst_test.sh
do_test()
{
tst_res TPASS "Passed"
TST_FOO=1
echo "$TST_PASS"
}
tst_run
# PATH is set to contain both path to tst_test.sh and d.sh
$ ./d.sh
test 1 TWARN : Reserved variable TST_FOO used!
test 1 TWARN : Reserved variable TST_PASS used!
test 1 TPASS : Passed
1
Summary:
passed 1
failed 0
skipped 0
warnings 2
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 25+ messages in thread* [LTP] new shell library
2016-10-12 13:17 ` Cyril Hrubis
@ 2016-10-12 14:54 ` Jan Stancek
2016-10-12 15:06 ` Cyril Hrubis
` (2 more replies)
0 siblings, 3 replies; 25+ messages in thread
From: Jan Stancek @ 2016-10-12 14:54 UTC (permalink / raw)
To: ltp
----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Wednesday, 12 October, 2016 3:17:26 PM
> Subject: Re: [LTP] new shell library
>
> Hi!
> > What about greping the test source as in [2] instead? Because that way
> > we can print error if the test source touches any of the internally used
> > variables as well. For instance if it tries to do anything with
> > TST_PASS/TST_FAIL/...
True.
>
> What about this one:
>
> https://github.com/metan-ucw/ltp/commit/445e3ae253bdd11f18ec12ccc74fe99eb582eeb6
Looks OK, chances of someone making MYTST_VARIABLE is quite small,
and we can always improve that regex if needed.
Couple notes:
- these seem to be missing: TST_DEVICE, TST_MODPATH, TST_CHECKPOINT_*, TST_NEEDS_CHECKPOINTS
- doc page mentions "TST_NEEDS_CMD" and "TST_NEEDS_CMDS", the latter looks like typo.
Regards,
Jan
>
> $ cat d.sh
>
> #!/bin/sh
> TST_ID="test"
> TST_TESTFUNC=do_test
> . tst_test.sh
>
> do_test()
> {
> tst_res TPASS "Passed"
>
> TST_FOO=1
>
> echo "$TST_PASS"
> }
>
> tst_run
>
> # PATH is set to contain both path to tst_test.sh and d.sh
> $ ./d.sh
> test 1 TWARN : Reserved variable TST_FOO used!
> test 1 TWARN : Reserved variable TST_PASS used!
> test 1 TPASS : Passed
> 1
>
> Summary:
> passed 1
> failed 0
> skipped 0
> warnings 2
>
> --
> Cyril Hrubis
> chrubis@suse.cz
>
^ permalink raw reply [flat|nested] 25+ messages in thread* [LTP] new shell library
2016-10-12 14:54 ` Jan Stancek
@ 2016-10-12 15:06 ` Cyril Hrubis
2016-10-13 15:43 ` Cyril Hrubis
2016-10-27 14:58 ` Cyril Hrubis
2 siblings, 0 replies; 25+ messages in thread
From: Cyril Hrubis @ 2016-10-12 15:06 UTC (permalink / raw)
To: ltp
Hi!
> Looks OK, chances of someone making MYTST_VARIABLE is quite small,
> and we can always improve that regex if needed.
:)
> Couple notes:
> - these seem to be missing: TST_DEVICE, TST_MODPATH, TST_CHECKPOINT_*, TST_NEEDS_CHECKPOINTS
Right, added the TST_DEVICE and TST_MODPATH and updated the git repo.
The checkpoints are not implemented in the tst_test.sh yet, I will add
them later on.
> - doc page mentions "TST_NEEDS_CMD" and "TST_NEEDS_CMDS", the latter looks like typo.
It's TST_NEEDS_CMDS since you can pass string of commands separated by
whitespaces. I've fixed the typo, added an example to the documentation
and updated the repo.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] new shell library
2016-10-12 14:54 ` Jan Stancek
2016-10-12 15:06 ` Cyril Hrubis
@ 2016-10-13 15:43 ` Cyril Hrubis
2016-10-27 14:58 ` Cyril Hrubis
2 siblings, 0 replies; 25+ messages in thread
From: Cyril Hrubis @ 2016-10-13 15:43 UTC (permalink / raw)
To: ltp
Hi!
Just FYI, I've added support for positional arguments, since many shell
tests use these, and tried to convert two of these. See:
https://github.com/metan-ucw/ltp/commit/3860c62e4f1ca6aa2f352ec50dd7087fa75b4487
https://github.com/metan-ucw/ltp/commit/5203cc54bbc64125a1c18ab93a4d13347609dc6e
https://github.com/metan-ucw/ltp/commit/3b7092f76e183627b3bb028bd5a8064e518d469a
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] new shell library
2016-10-12 14:54 ` Jan Stancek
2016-10-12 15:06 ` Cyril Hrubis
2016-10-13 15:43 ` Cyril Hrubis
@ 2016-10-27 14:58 ` Cyril Hrubis
2016-10-31 10:03 ` Jan Stancek
2 siblings, 1 reply; 25+ messages in thread
From: Cyril Hrubis @ 2016-10-27 14:58 UTC (permalink / raw)
To: ltp
Hi!
Up to date version is at usuall place[1]. I've managed to clean up most
of the tests in commands (four more to go) and I think that I ate enough
of my own dog food and, at this point, the tst_test.sh library should be
reasonably stable.
There is one change I'm not 100% sure of [2]. This is about ROD,
EXPECT_PASS and EXPECT_FAIL. These are all implemented using one common
funcntion and the $@ gets passed to it then gets parser in a loop in
order to implement redirection and that is where the paramteres are
expanded several times which breaks things horribly. As far as I can
tell this is impossible to implement correctly in shell. For instance
things like: ROD awk '{print $1}' wouldn't work with ROD_BASE as a shell
fuction. As a result I've reimplemented the ROD_BASE as tst_rod in C,
which works mostly fine. It's just stdin/stdout/stderr redirection +
execvp() (and fallback to "/bin/sh -s" for shell buildins). The only
thing that does not work are shell buildins that require to be executed
in the context of current shell, such as 'wait' or 'jobs'. I'm pondering
if it is worth of catching these two special cases in the tst_test.sh
shell library to fix that issue. Any kind of feedback to this is warmly
welcomed.
The rest of the changes should be more or less fine. The shell testcases
now support looping parameter and have well defined way of adding test
specific ones. What is not yet implemented is support for timeouts, that
could be easily done later since that is more or less orthogonal to the
current work.
All in all I would love to have a second pair of eyes (or even more!) to
check the code now.
[1] https://github.com/metan-ucw/ltp/commits/master
[2] https://github.com/metan-ucw/ltp/commit/61bebd11c6f956a7b9fcdf319ea6f4a47f5de35a
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 25+ messages in thread* [LTP] new shell library
2016-10-27 14:58 ` Cyril Hrubis
@ 2016-10-31 10:03 ` Jan Stancek
2016-10-31 10:39 ` Cyril Hrubis
2016-11-14 13:12 ` Cyril Hrubis
0 siblings, 2 replies; 25+ messages in thread
From: Jan Stancek @ 2016-10-31 10:03 UTC (permalink / raw)
To: ltp
----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Thursday, 27 October, 2016 4:58:06 PM
> Subject: Re: [LTP] new shell library
>
> Hi!
> Up to date version is at usuall place[1]. I've managed to clean up most
> of the tests in commands (four more to go) and I think that I ate enough
> of my own dog food and, at this point, the tst_test.sh library should be
> reasonably stable.
>
> There is one change I'm not 100% sure of [2]. This is about ROD,
> EXPECT_PASS and EXPECT_FAIL. These are all implemented using one common
> funcntion and the $@ gets passed to it then gets parser in a loop in
> order to implement redirection and that is where the paramteres are
> expanded several times which breaks things horribly. As far as I can
> tell this is impossible to implement correctly in shell. For instance
> things like: ROD awk '{print $1}' wouldn't work with ROD_BASE as a shell
> fuction. As a result I've reimplemented the ROD_BASE as tst_rod in C,
> which works mostly fine. It's just stdin/stdout/stderr redirection +
> execvp() (and fallback to "/bin/sh -s" for shell buildins). The only
> thing that does not work are shell buildins that require to be executed
> in the context of current shell, such as 'wait' or 'jobs'. I'm pondering
> if it is worth of catching these two special cases in the tst_test.sh
> shell library to fix that issue. Any kind of feedback to this is warmly
> welcomed.
I don't recall original ROD email thread, but did we consider eval and
passing entire command as single argument? For most commands it would
seem to be as simple as adding quotes around, awk though needs some
escaping:
#!/bin/sh
ROD_BASE()
{
eval $@
}
ROD()
{
ROD_BASE "$@"
if [ $? -ne 0 ]; then
tst_brk TBROK "$@ failed"
fi
}
ROD_SILENT()
{
ROD_BASE "$@" > /dev/null 2>&1
if [ $? -ne 0 ]; then
tst_brk TBROK "$@ failed"
fi
}
sleep 1 &
ROD jobs
ROD "ps | awk '{print \$1}' > tempfile"
ROD_SILENT "whoami >> tempfile"
ROD_SILENT ls
ROD ls -la
>
> The rest of the changes should be more or less fine. The shell testcases
> now support looping parameter and have well defined way of adding test
> specific ones. What is not yet implemented is support for timeouts, that
> could be easily done later since that is more or less orthogonal to the
> current work.
>
> All in all I would love to have a second pair of eyes (or even more!) to
> check the code now.
>
> [1] https://github.com/metan-ucw/ltp/commits/master
> [2]
> https://github.com/metan-ucw/ltp/commit/61bebd11c6f956a7b9fcdf319ea6f4a47f5de35a
>
> --
> Cyril Hrubis
> chrubis@suse.cz
>
^ permalink raw reply [flat|nested] 25+ messages in thread* [LTP] new shell library
2016-10-31 10:03 ` Jan Stancek
@ 2016-10-31 10:39 ` Cyril Hrubis
2016-11-14 13:12 ` Cyril Hrubis
1 sibling, 0 replies; 25+ messages in thread
From: Cyril Hrubis @ 2016-10-31 10:39 UTC (permalink / raw)
To: ltp
Hi!
> I don't recall original ROD email thread, but did we consider eval and
> passing entire command as single argument? For most commands it would
> seem to be as simple as adding quotes around, awk though needs some
> escaping:
>
> #!/bin/sh
>
> ROD_BASE()
> {
> eval $@
> }
>
> ROD()
> {
> ROD_BASE "$@"
> if [ $? -ne 0 ]; then
> tst_brk TBROK "$@ failed"
> fi
> }
>
> ROD_SILENT()
> {
> ROD_BASE "$@" > /dev/null 2>&1
> if [ $? -ne 0 ]; then
> tst_brk TBROK "$@ failed"
> fi
> }
>
> sleep 1 &
> ROD jobs
> ROD "ps | awk '{print \$1}' > tempfile"
I wanted to avoid putting the whole command line into "" as well as the
need to escape $ signs inside of '', etc.
For instance if I wanted to print literal '\n' I would have to do:
ROD echo "\\\\n"
whereas with the C helper we can do just:
ROD echo "\\n"
I'm not saying that the C binary is the best solution to the problem but
I do not thing that using eval does not seem to be much better, it's
certainly shorter to implement though.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 25+ messages in thread* [LTP] new shell library
2016-10-31 10:03 ` Jan Stancek
2016-10-31 10:39 ` Cyril Hrubis
@ 2016-11-14 13:12 ` Cyril Hrubis
2016-11-22 6:50 ` Cyril Hrubis
1 sibling, 1 reply; 25+ messages in thread
From: Cyril Hrubis @ 2016-11-14 13:12 UTC (permalink / raw)
To: ltp
Hi!
I've added a few patches that remove tst_kvercmp in favor of tst_kvcmp
into the repository[1]. These are more or less trivial, the rest of the
patchset is the same, I only converted one more test (ar01) and keep
fixing conflicts so that it could be applied on the top of the latest
git tree.
Has anybody any feedback that would stop the inclusion of these patches?
If not I would like to get them into the git once I test that these
testcases can run fine from an installed tree so that we have enough
time to test the changes out before the next release.
[1] https://github.com/metan-ucw/ltp
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] new shell library
2016-11-14 13:12 ` Cyril Hrubis
@ 2016-11-22 6:50 ` Cyril Hrubis
2016-11-22 7:54 ` Jan Stancek
0 siblings, 1 reply; 25+ messages in thread
From: Cyril Hrubis @ 2016-11-22 6:50 UTC (permalink / raw)
To: ltp
Hi!
> [1] https://github.com/metan-ucw/ltp
I did a few runs of that ltp git repo on SLES and Debian and caught a
few typos and problems. Anything else I should do before we merge it
into main LTP git?
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] new shell library
2016-11-22 6:50 ` Cyril Hrubis
@ 2016-11-22 7:54 ` Jan Stancek
2016-11-22 8:02 ` Cyril Hrubis
0 siblings, 1 reply; 25+ messages in thread
From: Jan Stancek @ 2016-11-22 7:54 UTC (permalink / raw)
To: ltp
----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Tuesday, 22 November, 2016 7:50:33 AM
> Subject: Re: [LTP] new shell library
>
> Hi!
> > [1] https://github.com/metan-ucw/ltp
>
> I did a few runs of that ltp git repo on SLES and Debian and caught a
> few typos and problems. Anything else I should do before we merge it
> into main LTP git?
If you can wait a bit with merge, I'll run commands runtest on few RHEL
distros (from ancient to current) to double check.
Regards,
Jan
>
> --
> Cyril Hrubis
> chrubis@suse.cz
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] new shell library
2016-11-22 7:54 ` Jan Stancek
@ 2016-11-22 8:02 ` Cyril Hrubis
2016-11-22 10:47 ` Jan Stancek
0 siblings, 1 reply; 25+ messages in thread
From: Cyril Hrubis @ 2016-11-22 8:02 UTC (permalink / raw)
To: ltp
Hi!
> > I did a few runs of that ltp git repo on SLES and Debian and caught a
> > few typos and problems. Anything else I should do before we merge it
> > into main LTP git?
>
> If you can wait a bit with merge, I'll run commands runtest on few RHEL
> distros (from ancient to current) to double check.
Sounds good.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] new shell library
2016-11-22 8:02 ` Cyril Hrubis
@ 2016-11-22 10:47 ` Jan Stancek
2016-11-22 11:21 ` Cyril Hrubis
0 siblings, 1 reply; 25+ messages in thread
From: Jan Stancek @ 2016-11-22 10:47 UTC (permalink / raw)
To: ltp
----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Tuesday, 22 November, 2016 9:02:52 AM
> Subject: Re: [LTP] new shell library
>
> Hi!
> > > I did a few runs of that ltp git repo on SLES and Debian and caught a
> > > few typos and problems. Anything else I should do before we merge it
> > > into main LTP git?
> >
> > If you can wait a bit with merge, I'll run commands runtest on few RHEL
> > distros (from ancient to current) to double check.
>
> Sounds good.
Looks good. Only thing that failed was 'file' test on older distros,
because output of 'file' is slightly different for certain file types.
Regards,
Jan
>
> --
> Cyril Hrubis
> chrubis@suse.cz
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] new shell library
2016-11-22 10:47 ` Jan Stancek
@ 2016-11-22 11:21 ` Cyril Hrubis
0 siblings, 0 replies; 25+ messages in thread
From: Cyril Hrubis @ 2016-11-22 11:21 UTC (permalink / raw)
To: ltp
Hi!
> Looks good. Only thing that failed was 'file' test on older distros,
> because output of 'file' is slightly different for certain file types.
Hmm, I've seen that as well and fixed a few occurences.
Should we fix that before the patchset gets merged or will you fix that
afterwards in a follow up patch?
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] new shell library
@ 2016-11-22 12:20 Jan Stancek
2016-11-22 13:27 ` Cyril Hrubis
0 siblings, 1 reply; 25+ messages in thread
From: Jan Stancek @ 2016-11-22 12:20 UTC (permalink / raw)
To: ltp
----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: "Jan Stancek" <jstancek@redhat.com>
> Cc: ltp@lists.linux.it
> Sent: Tuesday, 22 November, 2016 12:21:49 PM
> Subject: Re: [LTP] new shell library
>
> Hi!
> > Looks good. Only thing that failed was 'file' test on older distros,
> > because output of 'file' is slightly different for certain file types.
>
> Hmm, I've seen that as well and fixed a few occurences.
>
> Should we fix that before the patchset gets merged or will you fix that
> afterwards in a follow up patch?
I'm attaching changes, which allow file_test to pass on RHEL5.6/6.0/6.9/7.3
and the rest we can fix if/when we encounter any.
If that works for you, then I suggest to add/squash this on top of your changes.
Regards,
Jan
>
> --
> Cyril Hrubis
> chrubis@suse.cz
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: rhel_file_test_changes.patch
Type: text/x-patch
Size: 1412 bytes
Desc: not available
URL: <http://lists.linux.it/pipermail/ltp/attachments/20161122/b6e9a0e4/attachment.bin>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [LTP] new shell library
2016-11-22 12:20 Jan Stancek
@ 2016-11-22 13:27 ` Cyril Hrubis
0 siblings, 0 replies; 25+ messages in thread
From: Cyril Hrubis @ 2016-11-22 13:27 UTC (permalink / raw)
To: ltp
Hi!
> I'm attaching changes, which allow file_test to pass on RHEL5.6/6.0/6.9/7.3
> and the rest we can fix if/when we encounter any.
>
> If that works for you, then I suggest to add/squash this on top of your changes.
Squashed to the commit, added your signed-of-by, rebased the patches and
pushed, uff.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2016-11-22 13:27 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-03 14:58 [LTP] new shell library Cyril Hrubis
2016-10-04 8:23 ` Jan Stancek
2016-10-04 8:45 ` Cyril Hrubis
2016-10-04 9:02 ` Jan Stancek
2016-10-04 9:35 ` Cyril Hrubis
2016-10-04 9:54 ` Jan Stancek
2016-10-04 11:50 ` Cyril Hrubis
2016-10-12 10:08 ` Cyril Hrubis
2016-10-12 10:32 ` Jan Stancek
2016-10-12 12:36 ` Cyril Hrubis
2016-10-12 13:17 ` Cyril Hrubis
2016-10-12 14:54 ` Jan Stancek
2016-10-12 15:06 ` Cyril Hrubis
2016-10-13 15:43 ` Cyril Hrubis
2016-10-27 14:58 ` Cyril Hrubis
2016-10-31 10:03 ` Jan Stancek
2016-10-31 10:39 ` Cyril Hrubis
2016-11-14 13:12 ` Cyril Hrubis
2016-11-22 6:50 ` Cyril Hrubis
2016-11-22 7:54 ` Jan Stancek
2016-11-22 8:02 ` Cyril Hrubis
2016-11-22 10:47 ` Jan Stancek
2016-11-22 11:21 ` Cyril Hrubis
-- strict thread matches above, loose matches on Subject: below --
2016-11-22 12:20 Jan Stancek
2016-11-22 13:27 ` Cyril Hrubis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox