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

From: Mattias Andrée <maandree_AT_kth.se>
Date: Wed, 1 Aug 2018 22:12:54 +0200

On Wed, 1 Aug 2018 22:07:33 +0200
Mattias Andrée <maandree_AT_kth.se> wrote:

> On Wed, 1 Aug 2018 21:16:26 +0200
> Silvan Jegen <s.jegen_AT_gmail.com> wrote:
>
> > 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
>
> Hi again Silvan,
>
> >
> >
> > > 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...
>
> It's worse than that, nothing about echo is portable:
>
> - You don't know how backslashes are interpreted.
>
> - You don't know whether you get a new line at the end.
>
> - You don't know if -e is supported.
>
> - You don't know if -n is supported.
>
> >
> >
> > > * 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
>
> s/ed/sh/ # I guess you understood that, but I cannot stand not correcting it.
>
> > > 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?
>
> I am open to it, however I fear that it is not the simpler solution
> as it may require two implementations of the same functionality.

Sorry, I'm mixing things up, the idea is to completely replace
the C version of test-common, not just add another implementation.

>
> >
> >
> > > 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.
>
> I will make a sh reimplementation, but since I'm back at work
> now, it will take some time. In the meanwhile, why not enjoy
> my new painting.
>
> >
> >
> > Cheers,
> >
> > Silvan
>
> Have a nice evening,
> Mattias Andrée
Received on Wed Aug 01 2018 - 22:12:54 CEST