Skip to content
Permalink
Browse files

Make libast API tests hermetic

The libast API tests prior to this change simply ran in the current working
directory (the build dir) with the environment they inherited. That can
result in false positive failures. For example, due to two tests that
create a file with the same name.

Note that the API/sfio/talarm test fails a lot of the time with this
change. That is because it writes the following message to stderr
without exiting with a non-zero status:

    Line=45: System call was automatically resumed by the OS

This change causes such unexpected messages to be treated as a test
failure. Whereas the test framework prior to this change silently ignores
stderr output.

This also removes the special-casing of "*.out" pathnames since it caused
me problems when implementing this feature. That was due to creating a
*.out test file to confirm that if it didn't match the stdout of an API
test the conclusion is the test failed.

Finally, this renames to CDT API tests to avoid name collisions with
SFIO API tests of the same name. This allows me to remove the hack I
implemented a few weeks ago to ensure the test executables had unique
names to work around a Meson bug.

Resolves #1243
  • Loading branch information...
krader1961 committed Mar 27, 2019
1 parent dc2e8d0 commit e6498a2d95ece80b77c8b3cba5479d01029e3103
@@ -34,11 +34,5 @@
._*
Desktop.ini

# This is a special suffix. There is exactly one directory where we expect
# files with a `.out` extension: the ksh93 unit tests. So exclude it
# everywhere then allow it in the ksh93 unit test directory.
*.out
!src/cmd/ksh93/tests/*.out

# Artifacts of the build and test process that should not be committed.
build/
@@ -14,7 +14,7 @@ matrix:
compiler: gcc
env:
- DISTRO_TYPE=fedora
INSTALL_REQUIREMENTS="dnf repolist; dnf --allowerasing install -y gcc python3 meson sudo langpacks-zh_CN ed ncurses vi findutils which nmap-ncat expect git"
INSTALL_REQUIREMENTS="dnf repolist; dnf --allowerasing install -y gcc python3 meson sudo langpacks-zh_CN ed ncurses vi findutils which nmap-ncat expect git ksh"

This comment has been minimized.

Copy link
@siteshwar

siteshwar Mar 28, 2019

Collaborator

Does this change require ksh package to build now ? Why can't we use ksh built during previous step ?

This comment has been minimized.

Copy link
@siteshwar

siteshwar Mar 28, 2019

Collaborator

It also means I won't be able to run tests inside rpm package builds in future.

This comment has been minimized.

Copy link
@krader1961

krader1961 Mar 28, 2019

Author Collaborator

It requires the platform ksh because at the point the libast API tests are run we may or may not have built ksh. In theory we can probably structure the build rules so that the libast API tests depend on the ksh built by this project. I simply punted solving that problem into the future since we can reasonably expect any system running these tests to have a working ksh. An alternative solution is to making using the platform's ksh optional and if not found simply skip the libast API tests.

This comment has been minimized.

Copy link
@siteshwar

siteshwar Mar 28, 2019

Collaborator

Can we use C APIs instead of shell script to set up test environment ?

This comment has been minimized.

Copy link
@krader1961

krader1961 Mar 28, 2019

Author Collaborator

Sure, at a huge increase in complexity. Which is pretty hard to justify. It's not just setting up the environment (e.g., creating a temp dir, sanitizing the env vars, etc.). It's also checking that the API test produced the expected stdout/stderr output.

I don't understand how this dependency causes problems for "rpm package builds". That already requires meson, ninja, python, the compiler, and a bunch of tools. Why is adding ksh a problem? And even if it is a problem why can't we just make it an optional dependency and skip the libast API tests if it isn't satisfied?

This comment has been minimized.

Copy link
@krader1961

krader1961 Mar 28, 2019

Author Collaborator

Also, would it make any difference if the dependency was on bash? Heck, we could probably make the API test runner work with a legacy /bin/sh. Would that solve the problem I don't understand?

This comment has been minimized.

Copy link
@siteshwar

siteshwar Mar 28, 2019

Collaborator

Package builds are done in a clean environment and in future I would like to run tests inside every package build (For e.g. see here). Ksh build should not depend on ksh itself.

This comment has been minimized.

Copy link
@siteshwar

siteshwar Mar 28, 2019

Collaborator

I think it would be better if we can make test script a POSIX shell script.

This comment has been minimized.

Copy link
@krader1961

krader1961 Mar 28, 2019

Author Collaborator

I think it would be better if we can make test script a POSIX shell script.

That should already be the case. I don't think there is anything in this test runner script (or the one in the ksh source tree) that requires ksh specific features. In which case it should be possible to replace the use of ksh to run run_test.sh with some other POSIX shell. Pick one.

before_install:
- docker pull ${DISTRO_TYPE}

@@ -25,7 +25,7 @@ matrix:
compiler: gcc
env:
- DISTRO_TYPE=opensuse
INSTALL_REQUIREMENTS="zypper refresh; zypper in -y gcc python3 python3-pip ninja sudo glibc-locale glibc-devel ed ncurses-utils vim findutils which netcat expect git; pip3 install meson==0.44.0"
INSTALL_REQUIREMENTS="zypper refresh; zypper in -y gcc python3 python3-pip ninja sudo glibc-locale glibc-devel ed ncurses-utils vim findutils which netcat expect git ksh; pip3 install meson==0.44.0"
before_install:
- docker pull ${DISTRO_TYPE}

@@ -35,7 +35,7 @@ matrix:
compiler: gcc
env:
- DISTRO_TYPE=ubuntu
INSTALL_REQUIREMENTS="apt-get update; DEBIAN_FRONTEND=noninteractive apt-get install -yq gcc python3 sudo locales ed vim python3-pip ninja-build findutils debianutils netcat-openbsd expect git; pip3 install meson==0.44.0; locale-gen en_US.UTF-8"
INSTALL_REQUIREMENTS="apt-get update; DEBIAN_FRONTEND=noninteractive apt-get install -yq gcc python3 sudo locales ed vim python3-pip ninja-build findutils debianutils netcat-openbsd expect git ksh; pip3 install meson==0.44.0; locale-gen en_US.UTF-8"
before_install:
- docker pull ${DISTRO_TYPE}

@@ -45,7 +45,7 @@ matrix:
compiler: gcc
env:
- DISTRO_TYPE=debian
INSTALL_REQUIREMENTS="apt-get update; DEBIAN_FRONTEND=noninteractive apt-get install -yq gcc python3 python3-pip ninja-build sudo locales ed vim procps findutils debianutils netcat-openbsd expect git; pip3 install meson==0.44.0; sed -i '/# en_US.UTF-8 UTF-8/s/..//' /etc/locale.gen && locale-gen"
INSTALL_REQUIREMENTS="apt-get update; DEBIAN_FRONTEND=noninteractive apt-get install -yq gcc python3 python3-pip ninja-build sudo locales ed vim procps findutils debianutils netcat-openbsd expect git ksh; pip3 install meson==0.44.0; sed -i '/# en_US.UTF-8 UTF-8/s/..//' /etc/locale.gen && locale-gen"
before_install:
- docker pull ${DISTRO_TYPE}

@@ -57,7 +57,7 @@ matrix:
compiler: gcc
env:
- DISTRO_TYPE=i386/ubuntu
INSTALL_REQUIREMENTS="apt-get update; DEBIAN_FRONTEND=noninteractive apt-get install -yq gcc python3 sudo locales ed vim python3-pip ninja-build findutils debianutils netcat-openbsd expect git; pip3 install meson==0.44.0; locale-gen en_US.UTF-8"
INSTALL_REQUIREMENTS="apt-get update; DEBIAN_FRONTEND=noninteractive apt-get install -yq gcc python3 sudo locales ed vim python3-pip ninja-build findutils debianutils netcat-openbsd expect git ksh; pip3 install meson==0.44.0; locale-gen en_US.UTF-8"
before_install:
- docker pull ${DISTRO_TYPE}

@@ -1,15 +1,14 @@
aso_test_files =['taso.c', 'tlock.c']
test_dir = meson.current_source_dir()
tests = ['taso', 'tlock']

incdir = include_directories('..',
'../../include/')
incdir = include_directories('..', '../../include/')

foreach file: aso_test_files
test_target = executable('aso_' + file, file, c_args: shared_c_args,
include_directories: [configuration_incdir, incdir],
link_with: [libast, libenv],
link_args: ['-lpthread'],
install: false)
# TODO: Figure out how to make these tests more efficient so we don't need such an absurdly long
# timeout in order to keep these tests from timing out on OpenBSD. See issue #483.
test('API/aso/' + file, test_target)
foreach test_name: tests
test_target = executable(
test_name, test_name + '.c',
c_args: shared_c_args,
include_directories: [configuration_incdir, incdir],
link_with: [libast, libenv],
install: false)
test('API/aso/' + test_name, ksh_exe, args: [test_driver, test_target, test_dir])
endforeach
@@ -1,28 +1,21 @@
# Each entry in `all_tests` is an array of one or two elements. The first
# element is the test name. The second is an optional timeout if the default
# timeout of 30 seconds is too short. Try to keep the list sorted.
default_timeout = 30
test_dir = meson.current_source_dir()

# TODO: Enable these tests when they are fixed to work reliably. At the moment these
# timeout or fail on most platforms:
# ['tsafehash.c', 120], ['tsafetree.c', 120],
cdt_test_files = [ ['tannounce.c'], ['tbags.c'], ['tdeque.c'], ['tevent.c'],
['tinstall.c'], ['tlist.c'], ['tobag.c'], ['tqueue.c'],
['trhbags.c'], ['tsearch.c'],
['tshare.c'], ['tstack.c'], ['tstringset.c'], ['tuser.c'],
['tvthread.c'], ['twalk.c'], ['tview.c'], ['trehash.c'],
]
tests = ['tannounce', 'tbags', 'tdeque', 'tdict', 'tdtstack', 'tevent', 'tinstall', 'tlist',
'tobag', 'tqueue', 'trhbags', 'tsearch', 'tstringset', 'tuser', 'tvthread', 'twalk',
'tview', 'trehash']

incdir = include_directories('..',
'../../include/')
incdir = include_directories('..', '../../include/')

foreach testspec: cdt_test_files
testname = testspec[0]
timeout = (testspec.length() == 2) ? testspec[1] : default_timeout
test_target = executable('ctd_' + testname, testname, c_args: shared_c_args,
include_directories: [configuration_incdir, incdir],
link_with: [libast, libenv],
link_args: ['-lpthread'],
install: false)
test('API/cdt/' + testname, test_target, timeout: timeout)
foreach test_name: tests
test_target = executable(
test_name, test_name + '.c',
c_args: shared_c_args,
include_directories: [configuration_incdir, incdir],
link_with: [libast, libenv],
link_args: ['-lpthread'],
install: false)
test('API/cdt/' + test_name, ksh_exe, args: [test_driver, test_target, test_dir])
endforeach
File renamed without changes.
File renamed without changes.
@@ -1,3 +1,6 @@
ksh_exe = find_program('ksh')
test_driver = join_paths(meson.current_source_dir(), 'run_test.sh')

subdir('aso')
subdir('cdt')
subdir('misc')
@@ -1,11 +1,12 @@
string_test_files = [ 'opt.c', 'strelapsed.c' ]
test_dir = meson.current_source_dir()
tests = ['opt']

incdir = include_directories('..',
'../../include/')
incdir = include_directories('..', '../../include/')

foreach file: string_test_files
foreach test_name: tests
test_target = executable(
'misc_' + file, file, c_args: shared_c_args,
test_name, test_name + '.c',
c_args: shared_c_args,
include_directories: [configuration_incdir, incdir],
link_with: [libast, libenv],
install: false)
@@ -14,7 +15,5 @@ foreach file: string_test_files
# For the moment we only build the programs to verify they can be built and to enable linting
# them. Running the tests will require converting the *.rt and *.tst files into scripts.
#
if file == 'base64.c'
test('API/misc/' + file, test_target)
endif
# test('API/misc/' + test_name, ksh_exe, args: [test_driver, test_target, test_dir])
endforeach

This file was deleted.

@@ -1,14 +1,15 @@
path_test_files = [ 'pathaccess.c', 'pathbin.c', 'pathcanon.c', 'pathcat.c',
'pathexists.c', 'pathgetlink.c', 'pathprog.c', 'pathpath.c',
'pathshell.c', 'pathstat.c', 'pathtemp.c' ]
test_dir = meson.current_source_dir()
tests = ['pathaccess', 'pathbin', 'pathcanon', 'pathcat', 'pathexists', 'pathgetlink', 'pathprog',
'pathpath', 'pathshell', 'pathstat', 'pathtemp']

incdir = include_directories('..',
'../../include/')
incdir = include_directories('..', '../../include/')

foreach file: path_test_files
test_target = executable('path_' + file, file, c_args: shared_c_args,
include_directories: [configuration_incdir, incdir],
link_with: [libast, libenv],
install: false)
test('API/path/' + file, test_target)
foreach test_name: tests
test_target = executable(
test_name, test_name + '.c',
c_args: shared_c_args,
include_directories: [configuration_incdir, incdir],
link_with: [libast, libenv],
install: false)
test('API/path/' + test_name, ksh_exe, args: [test_driver, test_target, test_dir])
endforeach
@@ -0,0 +1,134 @@
# This is loosely based on the src/cmd/ksh93/tests/util/run_test.sh script.
#
# This helps ensure each API test runs in a unique temp dir that is automatically cleaned up when
# the test successfully terminates.

#
# Make sure when called via `meson test` we've got the expected args.
#
function log_info {
typeset lineno=$1
print -r "<I> run_test[$lineno]: ${@:2}"
}
alias log_info='log_info $LINENO'

function log_warning {
typeset lineno=$1
print -u2 -r "<W> run_test[$lineno]: ${@:2}"
}
alias log_warning='log_warning $LINENO'

function log_error {
typeset lineno=$1
print -u2 -r "<E> run_test[$lineno]: ${@:2}"
}
alias log_error='log_error $LINENO'

if [[ $# -ne 2 ]]
then
log_error "Expected two args (the API test binary pathname and test name), got $#: $@"
exit 99
fi

export BUILD_DIR=$PWD

x=${1##*/}
readonly test_name=${x%.c}
readonly api_binary=$BUILD_DIR/$1
readonly test_src_dir=$2

#
# A test may need to alter its behavior based on the OS we're running on.
#
export OS_NAME=$(uname -s)

#
# Setup the environment for the unit test.
#

#
# Create a temp dir and make it the CWD for the unit test. It will be removed by the unit test
# postscript if there are no test failures.
#
# NOTE: We deliberately includle a space char in the pathname to help ensure we don't introduce
# tests that break when that is true. See issue #166 and #158.
#
# The use of `mktemp -dt` isn't ideal as it has slightly different meaning on BSD and GNU. But for
# our purposes that doesn't matter. It simply means the temp file name will contain the X's on a BSD
# system.
#
export TEST_DIR=$(mktemp -dt api.${test_name}.XXXXXX) ||
{ log_error "mktemp -dt failed"; exit 99; }
cd $TEST_DIR || { print -u2 "<E> 'cd $TEST_DIR' failed with status $?"; exit 99; }
log_info "TEST_DIR=$TEST_DIR"

#
# Make sure we search for external commands in the temporary test dir, then the test source dir,
# then the shell auxiliary commands, before any other directory in $PATH.
#
# This helps ensure that if the test creates a script file named "zzz" and there is an executable
# external command of the same name in PATH that we use the command created by the unit test.
# See issue #429.
#
export ORIG_PATH=$PATH
export SAFE_PATH="$TEST_DIR:$TEST_DIR/space dir:$test_src_dir:$BUILD_DIR/src/cmd/builtin"
export FULL_PATH=$SAFE_PATH:$ORIG_PATH
export PATH=$FULL_PATH

#
# We don't want the tests to modify the command history and the like of the user running the tests
# or be affected by the user's home dir.
#
mkdir "$TEST_DIR/home"
mkdir "$TEST_DIR/space dir"
export HOME=$TEST_DIR/home

# Run a libast API test.
function run_api {
$api_binary >$test_name.out 2>$test_name.err
exit_status=$?

if [[ -e $test_src_dir/$test_name.out ]]
then
if ! diff -q $test_src_dir/$test_name.out $test_name.out >/dev/null
then
log_error "Stdout for $test_name had unexpected differences:"
diff -U3 $test_src_dir/$test_name.out $test_name.out >&2
exit_status=1
fi
elif [[ -s $test_name.out ]]
then
log_error "Stdout for $test_name should have been empty:"
cat $test_name.out >&2
exit_status=1
fi

if [[ -e $test_src_dir/$test_name.err ]]
then
if ! diff -q $test_src_dir/$test_name.err $test_name.err >/dev/null
then
log_error "Stderr for $test_name had unexpected differences:"
diff -U3 $test_src_dir/$test_name.err $test_name.err >&2
exit_status=1
fi
elif [[ -s $test_name.err ]]
then
log_error "Stderr for $test_name should have been empty:"
cat $test_name.err >&2
exit_status=1
fi

if [[ $exit_status -eq 0 ]]
then
# We only remove the temp test dir if the test is successful. Otherwise we leave it since
# it may contain useful clues about why the test failed.
cd /tmp
rm -rf $TEST_DIR
fi

return $exit_status
}

run_api
status=$?
exit $status

0 comments on commit e6498a2

Please sign in to comment.
You can’t perform that action at this time.