Skip to content

Commit

Permalink
ci: Address PR comments
Browse files Browse the repository at this point in the history
* We can't simply check if there are more than 4 open file descriptors
    * There are tests who intentionally close a std, and that will lead
      the total counts to 2.
    * One way is to check if open fds are one more than n std, but that
      information is not in older valgrind.
    * If there is no fd leaks, then the FILE DESCRIPTORS: will always be
      followed by the fd pointing to LastDynamicAnalysis log file
        * The test will check for this to determine if there are any
	  leaking fds
* Change testing for regexes
    * We only have one regex now, so delete those tests for previous
      regexes
  • Loading branch information
Boquan Fang committed Oct 22, 2024
1 parent 39456d8 commit 5158969
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 86 deletions.
76 changes: 25 additions & 51 deletions codebuild/bin/s2n_open_fds_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,10 @@
import re
import sys

# Allow stdin, stdout, stderr, and the file descriptor for dynamic analysis log to remain open at exit
ACCEPTABLE_OPEN_FDS = 4
# Exit code matches the valgrind memory loss exit code
# Exit code matches the valgrind memory loss error code
ERROR_EXIT_CODE = 123

analysis_file_pattern = re.compile(r"^LastDynamicAnalysis.*")
# This regular expression captures valgrind 3.13 and valgrind 3.18+ log
fd_pattern = re.compile(r"FILE DESCRIPTORS: \d+ open(?: \(\d+ std\))? at exit.$")
error_message_start_pattern = re.compile(r"^Running .*/s2n_.*_test.c.*")
error_message_end_pattern = re.compile(r"^<end of output>$")
analysis_file_pattern = re.compile(r"LastDynamicAnalysis.*")


def open_analysis_file(path):
Expand All @@ -27,66 +21,46 @@ def open_analysis_file(path):
return file


def print_error_message(file_name, error_message):
# Take the path to the failing test file
print(f"{file_name.split(' ')[1]} leaks file descriptor(s):\n")
print(''.join(error_message))
return 0


def read_analysis_file(file):
exit_code = 0
error_message = []
to_store = False
to_print = False
file_name = ""
for line in file:
start_match = error_message_start_pattern.search(line)
if start_match:
file_name = start_match.group()
"""
The FILE DESCRIPTORS string sometimes come on the same line
as the Running s2n_test.c. Hence, we need multiple check to handle
that corner case.
"""
open_fd_match = fd_pattern.search(line)
if open_fd_match:
# Take the number of open file descriptors and check against the acceptable amount
if int(re.findall(r"\d+", open_fd_match.group())[0]) > ACCEPTABLE_OPEN_FDS:
exit_code = ERROR_EXIT_CODE
to_print = True
# Only store information about leaking file descriptors
to_store = True
else:
to_store = False

if error_message_end_pattern.match(line):
if to_print:
print_error_message(file_name, error_message)
to_store = False
error_message.clear()
file_name = ""
lines = file.readlines()
for i in range(len(lines)):
if "<end of output>" in lines[i]:
to_print = False

if to_store:
if open_fd_match:
error_message.append(open_fd_match.group() + '\n')
continue
# Check if the line contains FILE DESCRITOPRS:
parts = lines[i].split("FILE DESCRIPTORS: ")
if len(parts) > 1:
# If a process doesn't leak file descriptors
# Then the next line will always be the file descriptor
# for LastDynamicAnalysis log file.
if not analysis_file_pattern.search(lines[i + 1]):
# Print a new line to separate the old output from the new output
print("")
to_print = True
print(lines[i][lines[i].find("FILE DESCRIPTORS: "):], end=" ")
exit_code = ERROR_EXIT_CODE
continue
else:
error_message.append(line)

to_print = False
if to_print:
print(lines[i], end=" ")
file.close()
return exit_code


def print_banner():
print("############################################################################")
print("################# Test for Leaking File Descriptors ########################")
print("############################################################################")


def main():
print_banner()
file = open_analysis_file(sys.argv[1])
return read_analysis_file(file)


if __name__ == '__main__':
print_banner()
sys.exit(main())
39 changes: 4 additions & 35 deletions tests/regex/s2n_regex_test.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import pytest

from codebuild.bin.s2n_open_fds_test import analysis_file_pattern, fd_pattern, error_message_start_pattern, error_message_end_pattern
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
# SPDX-License-Identifier: Apache-2.0
from codebuild.bin.s2n_open_fds_test import analysis_file_pattern


def test_analysis_file_pattern():
Expand All @@ -9,35 +9,4 @@ def test_analysis_file_pattern():


def test_analysis_file_pattern_incorrect_file_name():
assert analysis_file_pattern.match("Lastdynamicanalysis_20241017-1745.log") == None


def test_fd_pattern_valgrind_3_13():
assert fd_pattern.search("==6099== FILE DESCRIPTORS: 4 open at exit.").group(
) == "FILE DESCRIPTORS: 4 open at exit."


def test_fd_pattern_valgrind_3_18():
assert fd_pattern.search("==6099== FILE DESCRIPTORS: 4 open (3 std) at exit.").group(
) == "FILE DESCRIPTORS: 4 open (3 std) at exit."


def test_fd_pattern_invalid():
assert fd_pattern.match("==6129== Open AF_UNIX socket 19: <unknown>") == None


def test_error_message_start_pattern():
assert error_message_start_pattern.match("Running /codebuild/output/src1744391194/src/tests/unit/s2n_blob_test.c ... PASSED 45 tests").group(
) == "Running /codebuild/output/src1744391194/src/tests/unit/s2n_blob_test.c ... PASSED 45 tests"


def test_error_message_start_pattern_invalid():
assert error_message_start_pattern.match("==6099== <inherited from parent>") == None


def test_error_message_end_pattern():
assert error_message_end_pattern.match("<end of output>").group() == "<end of output>"


def test_error_message_end_pattern_invalid():
assert error_message_end_pattern.match("==6099== FILE DESCRIPTORS: 4 open (3 std) at exit.") == None
assert analysis_file_pattern.match("Lastdynamicanalysis_20241017-1745.log") is None

0 comments on commit 5158969

Please sign in to comment.