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 committed Nov 3, 2022
1 parent 92e238a commit 7f9e19f
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 36 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 @@ -193,7 +193,7 @@ def __read_test_struct(test, test_suite):
prog,
expect,
timeout,
before,
befores,
after,
test_suite,
kernel_min,
Expand Down
51 changes: 37 additions & 14 deletions tests/runtime/engine/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,8 @@ def run_test(test):
signal.signal(signal.SIGALRM, Runner.__handler)

p = None
before = None
befores = []
child_name = None
bpftrace = None
after = None
try:
Expand Down Expand Up @@ -195,22 +196,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(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 Down Expand Up @@ -275,8 +296,10 @@ def run_test(test):
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 @@ -240,7 +240,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 7f9e19f

Please sign in to comment.