Re: [hackers] [sbase][PATCH v2] Add tests for some utilities

From: Silvan Jegen <s.jegen_AT_gmail.com>
Date: Wed, 1 Aug 2018 21:16:26 +0200

On Wed, Aug 01, 2018 at 07:53:18PM +0200, Mattias Andrée wrote:
> Thank you for your time!

Thank you for all your work! :P


> The common code is 590 lines of code, including:
>
> * 102 lines of code related to identifying the error when the
> test fails.
>
> * 14 lines of code for properly killing processes on failure,
> abortion, and when a test case hangs.
>
> * 32 lines of code, plus 13 of the lines counted above,
> for supporting concurrent tests.
>
> This leaves 442 lines for the fundamental stuff and a few
> lines to support printing all errors rather than just the
> first error.
>
> Some note on your sh code (since you wrote “crappy and
> probably non-portable”, you are probably aware of this,
> but just in case):
>
> * `source` is a Bash synonym for the portable `.` command.

Yeah, that sounds familiar.
 

> * `echo` is unportable and `printf` should be used instead.

Didn't know that echo was not portable. Thought it was just a builtin
that should work the same everywhere. It's probably the flags that are
the issue...


> * I have never seen `&>` before, so I my guess is that it
> is a Bashism.

Yeah, seems likely. It's supposed to redirect both stderr and stdout. "sh"
did not complain about it but that doesn't mean much...


> * It looks like whichever file descriptor is not being
> inspected by `check_output` is printed the inherited
> file descriptor rather than to /dev/null.

Printing behaviour of the tests should looked at and fixed for sure.


> * I think it would be better to have something like:
>
> set -e
>
> run "test name" "./dirname //"
> check_stderr ""
> check_stdout / || check_stdout //
> check_exit 0
>
> Your sh code, with check_exit added, covers most current tests.
> However, it every time the usage output is modified it has to
> be change in the test case, which I guess is acceptable but
> undesirable. The tests that are currently implemented

I think that is working as intended. It's a behaviour change in the code
and should result in an error (unless we decide that we don't want to
test the usage output of course).


> and which need to be handled specially are:
>
> * sleep:
> This can be done with sh. With some adaption to the sh
> code, tests can also be done in parallel as it is in
> the C code.
>
> * test:
> test takes a lot of time to test, which is why multiple
> tests are run in parallel in the C code. Like tty(1),
> this test also requires the creation of terminals, but
> it also requires the creation of sockets.
>
> * time:
> Requires setitimer(3) and pause(3).
>
> * tty:
> This test requires the creation of terminals.
>
> * uname:
> Most of uname can be tested in ed, however, the output
> of uname with only one flag applied requires the uname
> system call.
>
> * whoami:
> The user name can be retrieved via $LOGNAME according
> to POSIX, however this requires that your login program
> actually sets it. Additionally (and this should be added
> to the test) when whoami is called from a program with
> setuid the owner of the program should be printed (i.e.
> the effective user), not real user which is stored in
> $LOGNAME.
>
> Additionally env, time, and yes requires identifying which signal
> the processed was killed by. I have never done this in sh, but I
> understand that it should be doable. time and env also requires a
> way to kill the process it runs with a specific signal. Furthermore
> the test for tty should include a case where stdin does not exist,
> which for the moment is not done. All tests excepts test and sleep
> fundamentally requires support for stdout, but they also use stderr
> for checking error support.
>
> These tests require all the features in the C code, except the
> extra functionally enumerated at the beginning of this mail and
> the support for binary data. These are tests we should focus on,
> whichever solution is found for them should be applied to the
> rest tests since those require almost only subset of the
> functionality. The only extra functionality other tests require

Thanks for compiling the list of tools to be handled in a special way.

I do feel that it would be better to find the simplest solution that
works for most of the tests. For the rest we can then decide how to
handle them in a minimal way (most likely some custom C code for each
one. Common parts should still be shared of course).


> is support for binary data; which I really don't think warrant
> a separate solution.
>
> The following utilities (from both sbase and ubase) still need
> tests, as well as the utilities listed under ubase's TODO. I
> have most of the work already done for the utilities marked
> with an asterisk.
>
> at awk bc cal
> cat chgrp chmod chown
> chroot chvt cksum* clear
> cmp cols comm cp
> cron ctrlaltdel cut date
> dd df diff dmesg
> du ed eject env
> expr fallocate find flock
> fold free freeramdisk fsfreeze
> getconf getty grep halt
> head hostname hwclock id
> insmod install join kill
> killall5 last lastlog ln
> logger login logname ls
> lsmod lsusb md5sum* mesg
> mkdir mkfifo mknod mkswap
> mktemp mount mountpoint mv
> nice nl nohup nologin*
> od pagesize passwd paste
> patch* pathchk pidof pivot_root
> printf ps pwd pwdx
> readahead readlink renice respawn
> rev rm rmdir rmmod
> sed seq* setsid sha1sum*
> sha224sum* sha256sum* sha384sum* sha512-224sum*
> sha512-256sum* sha512sum* sort split
> sponge stat strings stty
> su swaplabel swapoff swapon
> switch_root sync sysctl tail
> tar tee tftp touch
> tr truncate tsort umount
> uniq* unshare uptime uudecode
> uuencode vtallow watch wc
> which who xargs
>
> I think should look into what their tests will require, maybe
> even write some tests with optional test framework, especially
> for the tests that cannot be trivially tested in sh. We should
> also identify which of them cannot be automatically tested
> without a virtual machine, and write them down in the README,
> and possibly list of features to test. Once this is done, we
> can make a more informed discussion.
>
> For the moment I see two options: 1) write the tests mostly
> in sh and write special utilities in C where required, and
> 2) use the C code, but look for ways to improve it.

I feel like 1) is the way to go if we want to keep things as simple
as possible. The rest are special cases which can be handled in a
special way.

Would you be open for working on some (portable) shell code for the most
common tools?


> As I see it, the most complex parts of the C code are:
>
> * start_process:
> It's probably enough to split out some code to
> separate functions: the `if (in->flags & DATA)`,
> the dup2–close loops at the end.
>
> * wait_process:
> Instead of ready all file descriptors as fast as
> possible, the they could probably be read in order.
>
> * check_output_test:
> It's probably enough to add a few short comments
> and improve variable names.
>
> * print_failure:
> It's probably enough to add some empty lines add a
> few short comments.
>
> The other parts are pretty straight forward.

If we go with option 1) I would like to wait to see which C functionality
we would end up needing in the end. Looking at the C code I would postpone
until after that decision has been made.


Cheers,

Silvan

Received on Wed Aug 01 2018 - 21:16:26 CEST

This archive was generated by hypermail 2.3.0 : Wed Aug 01 2018 - 21:24:22 CEST