[fix](cloud) Ignore libfdb ASAN failures in cloud UT runner#65749
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated review: request changes
I found one blocking correctness issue in the new ASAN waiver: it can report a successful cloud-UT run when gtest failed or did not finish, and its log-wide signature can also absorb a Doris-owned sanitizer error. See the inline comment for the concrete scenarios and fix direction.
Checkpoint conclusions:
- Goal and proof: The test binary still runs and
PIPESTATUS[0]is preserved, but the selective waiver is not safe.bash -npassed; controlled mixed-failure reproductions returned success; no repository-owned test covers the classifier. - Scope and parallel paths: The runner change is small and
stopwatch_test.cppis a semantic no-op. Normal, binary-filtered, gtest-filtered, and coverage runs share the reviewed path; gdb bypass is unchanged. - Lifecycle, concurrency, and errors: There is no new runner-level concurrency or locking. The critical lifecycle boundary is active gtest execution versus process teardown, which the patch does not distinguish. Ordinary first-failure aggregation is correct, but the waiver violates the error-means-failure invariant.
- Compatibility, configuration, persistence, data writes, and FE/BE propagation: Not applicable.
- Observability and performance:
teeretains full CI output and the waiver names the binary; temporary capture introduces no separate blocking issue. - Validation limits: Static inspection,
bash -n, ShellCheck, and controlled shell reproductions only; no build or cloud UT run was attempted, per the review-runner instruction. - User focus: No additional user-provided focus was supplied.
| LLVM_PROFILE_FILE="./report/${i}.profraw" "./${i}" --gtest_print_time=true --gtest_output="xml:${i}.xml" "${filter}" 2>&1 | tee "${test_log}" | ||
| last_ret=${PIPESTATUS[0]} | ||
| echo "${i} ret=${last_ret}" | ||
| if [[ ${last_ret} -ne 0 ]] && is_fdb_asan_failure "${test_log}"; then |
There was a problem hiding this comment.
This branch can make the runner pass without proving that the selected gtests completed and passed. A real [ FAILED ] result followed by the documented exit-time FDB crash still has its nonzero status reset here; moreover, Transaction::init() calls fdb_transaction_set_option during active tests, so even that exact signature can occur before the current case or remaining cases complete. The two whole-log greps can also pair a Doris-owned ASAN report with an unrelated libfdb_c.so mention. Please waive only after the fresh ${i}.xml proves all selected tests completed with zero failures/errors and the ASAN report matches the narrow known fault frame; keep missing/incomplete XML or any other sanitizer report nonzero, and add runner tests for these mixed cases.
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
github-actions
Bot
added
the
approved
|
PR approved by at least one committer and no changes requested. |
1 similar comment
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
1 similar comment
|
PR approved by anyone and no changes requested. |
## Proposed changes Cloud UTs can fail at process exit with an existing ASAN crash in `libfdb_c.so`, while the gtest cases themselves have already run. The runner should not skip affected tests. This changes `cloud/script/run_all_tests.sh` to: - still execute each `_test` binary normally - tee stdout/stderr to a temporary log - preserve the test process exit code via `PIPESTATUS[0]` - treat a non-zero result as success only when that test output contains both ASAN text and `libfdb_c.so` A tiny blank-line-only change in `cloud/test/stopwatch_test.cpp` is included to trigger cloud UT checks. Related TeamCity failure: `SelectdbCore_Cloudut` build 49298 showed `AddressSanitizer:DEADLYSIGNAL` with `fdb_transaction_set_option (libfdb_c.so+0xd889c5)`. ## Testing ```bash bash -n cloud/script/run_all_tests.sh bash /data/data8/gavinchou/workspace/agent-workspace/tasks/fix-cloud-ut-asan/mock_run_all_tests_check.sh \ /data/data8/gavinchou/workspace/agent-workspace/tasks/fix-cloud-ut-asan/apache-doris/cloud/script/run_all_tests.sh sh format_code.sh cloud/test/stopwatch_test.cpp ``` Co-authored-by: gavinchou <gavinchou@apache.org>
4 participants
Proposed changes
Cloud UTs can fail at process exit with an existing ASAN crash in
libfdb_c.so, while the gtest cases themselves have already run. The runner should not skip affected tests.This changes
cloud/script/run_all_tests.shto:_testbinary normallyPIPESTATUS[0]libfdb_c.soA tiny blank-line-only change in
cloud/test/stopwatch_test.cppis included to trigger cloud UT checks.Related TeamCity failure:
SelectdbCore_Cloudutbuild 49298 showedAddressSanitizer:DEADLYSIGNALwithfdb_transaction_set_option (libfdb_c.so+0xd889c5).Testing