Skip to content

Commit

Permalink
tests: teach runtime runner about multiple before clauses
Browse files Browse the repository at this point in the history
As noted in bpftrace#2402, usdt flakiness was made better by
508538a but not fully fixed.

This commit is what I should have done all along: it allows the test
runner to parse and wait for multiple BEFORE clauses and thus ensures
the processes have started before the test runs.

There are two minor changes:

  1. The check for child processes is now `ps --ppid` based to
     eventually allow parallel process runs in the same environment.
  2. Because of the `ps` usage, the name check is now truncated to 15
     chars, which will fail if TASK_COMM_LEN is not 16. That looks like
     a constant in the kernel, so I think we're good.
  • Loading branch information
Delyan Kratunov authored and viktormalik committed Apr 20, 2023
1 parent 1392119 commit da44800
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 44 deletions.
8 changes: 4 additions & 4 deletions tests/runtime/engine/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class InvalidFieldError(Exception):
'prog',
'expect',
'timeout',
'before',
'befores',
'after',
'suite',
'kernel_min',
Expand Down Expand Up @@ -101,7 +101,7 @@ def __read_test_struct(test, test_suite):
prog = ''
expect = ''
timeout = ''
before = ''
befores = []
after = ''
kernel_min = ''
kernel_max = ''
Expand All @@ -128,7 +128,7 @@ def __read_test_struct(test, test_suite):
elif item_name == 'TIMEOUT':
timeout = int(line.strip(' '))
elif item_name == 'BEFORE':
before = line
befores.append(line)
elif item_name == 'AFTER':
after = line
elif item_name == 'MIN_KERNEL':
Expand Down Expand Up @@ -194,7 +194,7 @@ def __read_test_struct(test, test_suite):
prog,
expect,
timeout,
before,
befores,
after,
test_suite,
kernel_min,
Expand Down
67 changes: 45 additions & 22 deletions tests/runtime/engine/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,11 +158,13 @@ def run_test(test):
signal.signal(signal.SIGALRM, Runner.__handler)

p = None
before = None
befores = []
bpftrace = None
after = None
try:
result = None
timeout = False
output = ""

print(ok("[ RUN ] ") + "%s.%s" % (test.suite, test.name))
if test.requirement:
Expand Down Expand Up @@ -195,22 +197,42 @@ def run_test(test):
print(warn("[ SKIP ] ") + "%s.%s" % (test.suite, test.name))
return Runner.SKIP_FEATURE_REQUIREMENT_UNSATISFIED

if test.before:
before = subprocess.Popen(test.before, shell=True, preexec_fn=os.setsid)
waited=0
if test.befores:
for before in test.befores:
before = subprocess.Popen(before, shell=True, preexec_fn=os.setsid)
befores.append(before)

with open(os.devnull, 'w') as dn:
# This might not work for complicated cases, such as if
# a test program needs to accept arguments. It covers the
# current simple calls with no arguments
child_name = os.path.basename(test.before.split()[-1])
while subprocess.call(["pidof", child_name], stdout=dn, stderr=dn) != 0:
child_names = [os.path.basename(x.strip().split()[-1]) for x in test.befores]
child_names = sorted((x[:15] for x in child_names)) # cut to comm length

# Print the names of all of our children and look
# for the ones from BEFORE clauses
waited=0
while waited <= test.timeout:
children = subprocess.run(["ps", "--ppid", str(os.getpid()), "--no-headers", "-o", "comm"],
check=False,
capture_output=True)
if children.returncode == 0 and children.stdout:
lines = [line.decode('utf-8') for line in children.stdout.splitlines()]
lines = sorted((line.strip() for line in lines if line != 'ps'))
if lines == child_names:
break

time.sleep(0.1)
waited+=0.1
if waited > test.timeout:
raise TimeoutError('Timed out waiting for BEFORE %s ', test.before)

if waited > test.timeout:
raise TimeoutError(f'Timed out waiting for BEFORE(s) {test.befores}')

bpf_call = Runner.prepare_bpf_call(test)
if test.before and '{{BEFORE_PID}}' in bpf_call:
if test.befores and '{{BEFORE_PID}}' in bpf_call:
if len(test.befores) > 1:
raise ValueError(f"test has {len(test.befores)} BEFORE clauses but BEFORE_PID usage requires exactly one")

child_name = test.befores[0].strip().split()[-1]
child_name = os.path.basename(child_name)

childpid = subprocess.Popen(["pidof", child_name], stdout=subprocess.PIPE, universal_newlines=True).communicate()[0].split()[0]
bpf_call = re.sub("{{BEFORE_PID}}", str(childpid), bpf_call)
env = {
Expand All @@ -234,8 +256,6 @@ def run_test(test):

signal.alarm(ATTACH_TIMEOUT)

output = ""

while p.poll() is None:
nextline = p.stdout.readline()
output += nextline
Expand Down Expand Up @@ -268,15 +288,18 @@ def run_test(test):
os.killpg(os.getpgid(p.pid), signal.SIGTERM)
output += p.communicate()[0]
result = re.search(test.expect, output)
if not result:
print(fail("[ TIMEOUT ] ") + "%s.%s" % (test.suite, test.name))
print('\tCommand: %s' % bpf_call)
print('\tTimeout: %s' % test.timeout)
print('\tCurrent output: %s' % output)
return Runner.TIMEOUT

if not result:
print(fail("[ TIMEOUT ] ") + "%s.%s" % (test.suite, test.name))
print('\tCommand: %s' % bpf_call)
print('\tTimeout: %s' % test.timeout)
print('\tCurrent output: %s' % output)
return Runner.TIMEOUT
finally:
if before and before.poll() is None:
os.killpg(os.getpgid(before.pid), signal.SIGKILL)
if befores:
for before in befores:
if before.poll() is None:
os.killpg(os.getpgid(before.pid), signal.SIGKILL)

if bpftrace and bpftrace.poll() is None:
os.killpg(os.getpgid(p.pid), signal.SIGKILL)
Expand Down
3 changes: 2 additions & 1 deletion tests/runtime/usdt
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,8 @@ NAME "usdt probes - file based semaphore activation multi process"
RUN {{BPFTRACE}} runtime/scripts/usdt_file_activation_multiprocess.bt --usdt-file-activation
EXPECT found 2 processes
TIMEOUT 5
BEFORE ./testprogs/two_usdt_semaphore_tests
BEFORE ./testprogs/usdt_semaphore_test
BEFORE ./testprogs/usdt_semaphore_test
REQUIRES ./testprogs/usdt_semaphore_test should_not_skip

NAME "usdt probes - list probes by pid in separate mountns"
Expand Down
17 changes: 0 additions & 17 deletions tests/testprogs/two_usdt_semaphore_tests.c

This file was deleted.

0 comments on commit da44800

Please sign in to comment.