Skip to content

Commit

Permalink
Ensure that requires_root tests are run on main
Browse files Browse the repository at this point in the history
Summary:
The bazel_build_deps wasn't looking for `requires_root` even though
we group them together in the bazelrc. This change ensures that these tests
are now run in CI.

Test Plan:
Check Jenkins, ensure that the 4 tests that are `requires_root` but
not `manual` are now run.

Reviewers: zasgar, jamesbartlett, michelle, oazizi

Reviewed By: jamesbartlett

Signed-off-by: Vihang Mehta <vihang@pixielabs.ai>

Differential Revision: https://phab.corp.pixielabs.ai/D12781

GitOrigin-RevId: ee0f5c231f5772c9a5a3cb6817e31cef2dfc3902
  • Loading branch information
vihangm authored and copybaranaut committed Jan 13, 2023
1 parent 98fb09f commit aca4312
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 8 deletions.
1 change: 1 addition & 0 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ build:tsan --features=tsan
# Note that we are lumping tests that require root into the BPF tests below
# to minimize number of configs.
# If there are ever a lot of tests with requires_root, a new config is warranted.
# See also ci/bazel_build_deps.sh

# Note 2: BPF tests are limited to --jobs=4, because otherwise the parallel tests
# cause a lot of flakiness. In particular, many of the BPF tests deploy containers,
Expand Down
9 changes: 7 additions & 2 deletions ci/bazel_build_deps.sh
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,13 @@ cc_tests="kind(cc_.*, ${tests})"
go_buildables="kind(go_.*, ${buildables})"
go_tests="kind(go_.*, ${tests})"

bpf_buildables="attr('tags', 'requires_bpf', ${buildables})"
bpf_tests="attr('tags', 'requires_bpf', ${tests})"
# Note that we are lumping tests that require root into the BPF tests below
# to minimize number of configs.
# If there are ever a lot of tests with requires_root, a new config is warranted.
# See also .bazelrc

bpf_buildables="attr('tags', 'requires_bpf|requires_root', ${buildables})"
bpf_tests="attr('tags', 'requires_bpf|requires_root', ${tests})"

cc_bpf_buildables="kind(cc_.*, ${bpf_buildables})"
cc_bpf_tests="kind(cc_.*, ${bpf_tests})"
Expand Down
4 changes: 2 additions & 2 deletions src/stirling/source_connectors/perf_profiler/java/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
# SPDX-License-Identifier: Apache-2.0

load("//bazel:pl_build_system.bzl", "pl_cc_library", "pl_cc_test")
load("//src/stirling/source_connectors/perf_profiler/testing:testing.bzl", "agent_libs", "px_jattach", "stirling_profiler_java_args")
load("//src/stirling/source_connectors/perf_profiler/testing:testing.bzl", "agent_libs", "px_jattach", "stirling_profiler_attach_args")

package(default_visibility = ["//src/stirling:__subpackages__"])

Expand Down Expand Up @@ -51,7 +51,7 @@ pl_cc_test(
pl_cc_test(
name = "attach_test",
srcs = ["attach_test.cc"],
args = stirling_profiler_java_args,
args = stirling_profiler_attach_args,
data = ["//src/stirling/source_connectors/perf_profiler/testing/java:fib"] + agent_libs + [px_jattach],
tags = [
"no_asan",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,14 @@ using ::px::stirling::profiler::testing::GetPxJattachFlagValueForTesting;
using ::px::testing::BazelRunfilePath;
using ::testing::HasSubstr;

// TODO(jps): Fix and re-enable test.
// This test does the following:
// 1. Starts a target Java process (the fib app).
// 2. Uses the AgentAttach class to inject a JVMTI agent (our symbolization agent).
// 3. Finds the resulting symbol file and verifies that it has some symbols.
// Before doing any of the above, we setup some file paths for the target app,
// and for AgentAttach to find the JVMTI .so libs.
TEST(JavaAgentTest, ExpectedSymbolsTest) {
TEST(JavaAttachTest, DISABLED_ExpectedSymbolsTest) {
// Form the file name w/ user login to make it pedantically unique.
// Also, this is the same as in agent_test, so we keep the test logic consistent.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ TYPED_TEST(SymbolizerTest, UserSymbols) {

EXPECT_EQ(symbolize(kFooAddr), "test::foo()");
EXPECT_EQ(symbolize(kBarAddr), "test::bar()");
EXPECT_EQ(symbolize(2), std::string("0x0000000000000002"));
}

TEST_F(BCCSymbolizerTest, KernelSymbols) {
Expand All @@ -93,11 +92,12 @@ TEST_F(BCCSymbolizerTest, KernelSymbols) {
EXPECT_EQ(std::string(symbolize(kaddr)), kSymbolName);
}

// TODO(jps): Fix and re-enable test.
// This test uses a "fake" Java binary that creates a pre-populated (canned) symbol file.
// Because the fake Java process is named "java" the process is categorized as Java.
// The symbolizer finds the pre-existing symbol file, and early exits the attach process.
// This test expects to find known symbols at known addresses based on the canned symbol file.
TEST_F(BCCSymbolizerTest, JavaSymbols) {
TEST_F(BCCSymbolizerTest, DISABLED_DISABLED_JavaSymbols) {
PL_SET_FOR_SCOPE(FLAGS_stirling_profiler_java_agent_libs, GetAgentLibsFlagValueForTesting());
PL_SET_FOR_SCOPE(FLAGS_stirling_profiler_px_jattach_path, GetPxJattachFlagValueForTesting());
PL_SET_FOR_SCOPE(FLAGS_stirling_profiler_java_symbols, true);
Expand Down Expand Up @@ -212,8 +212,9 @@ TEST_F(BCCSymbolizerTest, DisableJavaSymbols) {
EXPECT_FALSE(JavaProfilingProcTracker::GetSingleton()->upids().contains(child_upid_1));
}

// TODO(jps): Fix and re-enable test.
// Java symbolizer does not attach if not enough space is available.
TEST_F(BCCSymbolizerTest, JavaNotEnoughSpaceAvailable) {
TEST_F(BCCSymbolizerTest, DISABLED_JavaNotEnoughSpaceAvailable) {
// Sets the tmpfs size, for the tmpfs volume that we will mount to /tmp in the target container.
// This size is too small for our Java symbolization libraries.
char const* const tmpfs_size_arg = "size=500K";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ px_jattach = "//src/stirling/source_connectors/perf_profiler/java/px_jattach:px_
agent_libs_arg = ",".join(["$(location %s)" % lib for lib in agent_libs])
px_jattach_arg = "$(location %s)" % px_jattach

stirling_profiler_attach_args = [
"-stirling_profiler_px_jattach_path %s" % px_jattach_arg,
]

stirling_profiler_java_args = [
"-stirling_profiler_java_agent_libs %s" % agent_libs_arg,
"-stirling_profiler_px_jattach_path %s" % px_jattach_arg,
Expand Down

0 comments on commit aca4312

Please sign in to comment.