On Wed, 1 Aug 2018 22:07:33 +0200 Mattias Andrée <maand...@kth.se> wrote:
> On Wed, 1 Aug 2018 21:16:26 +0200 > Silvan Jegen <s.je...@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
