Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C++/STL: recover from aborted tests #105

Merged
merged 3 commits into from
Mar 22, 2024
Merged

C++/STL: recover from aborted tests #105

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8226814
boost_test_parser: detect aborted tests
generalmimon Mar 18, 2024
dc9b6fc
C++/STL: recover from aborted tests
generalmimon Mar 18, 2024
25b2182
valgrind-cpp_stl: mirror run attempts in case of aborted tests
generalmimon Mar 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions ci-cpp_stl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,6 @@ CPP_TEST_OUT_DIR="$3"
echo 'Running Valgrind checks...'
if valgrind --version; then
./valgrind-cpp_stl "$SRC_DIR" "$CPP_TEST_OUT_DIR" || :

# For some obscure reason, valgrind generates this file as 0600 (owner-readable only).
# This disrupts invocation from Docker pipeline, as after return back from the Docker container
# this will become root-owned and root-only-readable file. We fix it here.
chmod 644 "$CPP_TEST_OUT_DIR/valgrind.xml"
else
echo 'Valgrind not found :-('

Expand Down
34 changes: 33 additions & 1 deletion valgrind-cpp_stl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,36 @@ CPP_TEST_OUT_DIR="$2"

OBJ_DIR="$SRC_DIR/bin"

valgrind --leak-check=full --xml=yes --xml-file="$CPP_TEST_OUT_DIR/valgrind.xml" "$OBJ_DIR/ks_tests"
run_valgrind() {
_attempt_no=$1
shift
_xml_file="$CPP_TEST_OUT_DIR/valgrind-${_attempt_no}.xml"
valgrind --leak-check=full --xml=yes --xml-file="${_xml_file}" "$OBJ_DIR/ks_tests" "$@"

# For some obscure reason, valgrind generates this file as 0600 (owner-readable only).
# This disrupts invocation from Docker pipeline, as after return back from the Docker container
# this will become root-owned and root-only-readable file. We fix it here.
chmod 644 "${_xml_file}"
}

read_next=1

run_attempt=0
set --

while [ -n "${read_next}" ]; do
run_attempt=$((run_attempt+1))
run_valgrind "${run_attempt}" "$@" || :

# if the `aborted_tests-{N}.txt` file is empty, then there were no aborted
# tests and it's the last attempt we need to process
read_next=
while IFS= read -r line; do
read_next=1
set -- "$@" "--run_test=!${line}"
done < "${CPP_TEST_OUT_DIR}/aborted_tests-${run_attempt}.txt"
done
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, one more argument towards re-doing the builds excluding offending file after run: we won't need to maintain such complex magic around valgrind runs.

Copy link
Member Author

@generalmimon generalmimon Mar 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to do that now either - as far as the contents of ci.json is concerned at the moment, only the log from the last Valgrind run (excluding any aborted tests) is copied to valgrind.xml and used by aggregate/convert_to_json. This is explained at the bottom of valgrind-cpp_stl (valgrind-cpp_stl:45-47):

# Currently, aggregate/convert_to_json only processes "valgrind.xml", so we
# make a copy with that name of the last (successful) attempt's log
cp -v "$CPP_TEST_OUT_DIR/valgrind-${run_attempt}.xml" "$CPP_TEST_OUT_DIR/valgrind.xml"

However, we'll likely eventually want to aggregate all valgrind-*.xml logs that this new version of valgrind-cpp_stl produces, because these Valgrind logs from running aborted tests contain all the details of the memory error causing the abort, as I mentioned above in #105 (comment).

So although the generated valgrind-*.xml logs are not used at the moment, they can be already helpful when manually inspecting the CI artifacts, and it doesn't hurt to have them available.


# Currently, aggregate/convert_to_json only processes "valgrind.xml", so we
# make a copy with that name of the last (successful) attempt's log
cp -v "$CPP_TEST_OUT_DIR/valgrind-${run_attempt}.xml" "$CPP_TEST_OUT_DIR/valgrind.xml"