Hey folks, I'm filing my first PR (<https://github...
# code-review
g
Hey folks, I'm filing my first PR (https://github.com/osquery/osquery/pull/8502), and I'm supposed to add some tests for the changes, which is good. Unfortunately, I can't seem to get the tests running without failure on my Mac, even on the
master
branch, I get 20 test failures :
Copy code
75% tests passed, 20 tests failed out of 81

Total Test time (real) = 1299.64 sec

The following tests FAILED:
	 13 - osquery_distributed_tests-test (Failed)
	 17 - osquery_tables_events_tests_fileeventstests-test (Failed)
	 18 - osquery_tables_networking_tests_networkingtablestests-test (Failed)
	 19 - osquery_tables_networking_tests_wifitests-test (Failed)
	 23 - osquery_tables_system_darwin_tests-test (SEGFAULT)
	 29 - osquery_remote_transports_remotetransportstlstests-test (Failed)
	 34 - osquery_config_tests-test (Failed)
	 35 - osquery_config_tests_packs-test (Failed)
	 37 - osquery_utils_aws_tests-test (Failed)
	 52 - osquery_filesystem_filesystemtests-test (Failed)
	 62 - plugins_config_tests_tlsconfigtests-test (Failed)
	 63 - plugins_config_parsers_tests_decoratorstests-test (Failed)
	 64 - plugins_config_parsers_tests_eventsparsertests-test (Failed)
	 65 - plugins_config_parsers_tests_filepathstests-test (Failed)
	 66 - plugins_config_parsers_tests_optionstests-test (Failed)
	 67 - plugins_config_parsers_tests_viewstests-test (SEGFAULT)
	 75 - plugins_logger_tests_tlsloggertests-test (Failed)
	 77 - plugins_remote_enroll_tlsenrolltests-test (Failed)
	 79 - tools_tests_testosqueryi (Failed)
	 81 - tests_integration_tables-test (Failed)
I build with the following commands:
Copy code
cmake -DCMAKE_OSX_DEPLOYMENT_TARGET=10.15 -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DOSQUERY_BUILD_TESTS=ON ..
cmake --build . -j $(sysctl -n hw.ncpu)
CTEST_OUTPUT_ON_FAILURE=1 cmake --build . --target test -j $(sysctl -n hw.ncpu)
I can attach a full log if it's relevant.
s
I suspect you might not have installed the python packages necessary for the tests. You want to see the
Step 1: Install macOS prerequisites
https://osquery.readthedocs.io/en/latest/development/building/ You have to install the "optional" (if not running tests) python packages.
If you still have issues, please do show the full error log, so we can see what's happening.
you can also run single tests with verbose output by name, via
ctest -V -R <testname>
Admittedly some that SEGFAULT there it's strange that python is involved at all.
g
I installed the python deps, but in a virtual env. Would that be an issue ? 🤔
the full output of the tests, ran with
CTEST_OUTPUT_ON_FAILURE=1
is attached:
s
Copy code
Cannot read --enroll_secret: [Errno 2] No such file or directory: '/Users/myself/src/osquery/build/test_configs/test_enroll_secret.txt'
This is one of the errors that happens when starting the python http server, necessary for some tests
is that file actually there?
It should be present when building everything
g
it is not. Should I have created it ? 🤔
s
Not you directly, the build system should've. Can you please show what's the output of the two commands:
Copy code
cmake -DCMAKE_OSX_DEPLOYMENT_TARGET=10.15 -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DOSQUERY_BUILD_TESTS=ON ..
cmake --build . -j $(sysctl -n hw.ncpu)
if you re-run them again?
g
first one:
Copy code
cmake -DCMAKE_OSX_DEPLOYMENT_TARGET=10.15 -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DOSQUERY_BUILD_TESTS=ON ..
-- osquery version: 5.14.1-24-g25236d899-dirty
-- Found ccache: /opt/homebrew/bin/ccache
-- Found clang-format: /opt/homebrew/bin/clang-format
-- Build type: Debug
-- Shared libraries: OFF
-- Importing: source/lz4
-- Importing: source/augeas/gnulib
-- Importing: source/augeas
-- Importing formula: formula/openssl
CMake Warning (dev) at /opt/homebrew/share/cmake/Modules/ExternalProject/shared_internal_commands.cmake:1276 (message):
  The DOWNLOAD_EXTRACT_TIMESTAMP option was not given and policy CMP0135 is
  not set.  The policy's OLD behavior will be used.  When using a URL
  download, the timestamps of extracted files should preferably be that of
  the time of extraction, otherwise code that depends on the extracted
  contents might not be rebuilt if the URL changes.  The OLD behavior
  preserves the timestamps from the archive instead, but this is usually not
  what you want.  Update your project to the NEW behavior or specify the
  DOWNLOAD_EXTRACT_TIMESTAMP option with a value of true to avoid this
  robustness issue.
Call Stack (most recent call first):
  /opt/homebrew/share/cmake/Modules/ExternalProject.cmake:3041 (_ep_add_download_command)
  libraries/cmake/formula/openssl/CMakeLists.txt:202 (ExternalProject_Add)
  libraries/cmake/formula/openssl/CMakeLists.txt:238 (opensslMain)
This warning is for project developers.  Use -Wno-dev to suppress it.

-- Importing: source/boost
-- osquery: boost::context is using the fcontext backend
-- Importing: source/bzip2
-- Importing: source/gflags
-- Importing: source/glog
-- Importing: source/googletest
CMake Deprecation Warning at libraries/cmake/source/googletest/src/CMakeLists.txt:4 (cmake_minimum_required):
  Compatibility with CMake < 3.10 will be removed from a future version of
  CMake.

  Update the VERSION argument <min> value.  Or, use the <min>...<max> syntax
  to tell CMake that the project requires at least <min> but has been updated
  to work with policies introduced by <max> or earlier.


CMake Deprecation Warning at libraries/cmake/source/googletest/src/googlemock/CMakeLists.txt:45 (cmake_minimum_required):
  Compatibility with CMake < 3.10 will be removed from a future version of
  CMake.

  Update the VERSION argument <min> value.  Or, use the <min>...<max> syntax
  to tell CMake that the project requires at least <min> but has been updated
  to work with policies introduced by <max> or earlier.


CMake Deprecation Warning at libraries/cmake/source/googletest/src/googletest/CMakeLists.txt:56 (cmake_minimum_required):
  Compatibility with CMake < 3.10 will be removed from a future version of
  CMake.

  Update the VERSION argument <min> value.  Or, use the <min>...<max> syntax
  to tell CMake that the project requires at least <min> but has been updated
  to work with policies introduced by <max> or earlier.


-- Importing: source/libarchive
-- Importing: source/libmagic
-- Importing: source/librdkafka
-- Importing: source/linenoise-ng
-- Importing: source/lzma
-- Importing: source/rapidjson
-- Importing: source/rocksdb
-- Importing: source/sleuthkit
-- Importing: source/sqlite
-- Importing: source/thrift
-- Importing: source/yara
-- Importing: source/zlib
-- Importing: source/zstd
-- Importing: source/aws-sdk-cpp
-- Could NOT find cppcheck (missing: CPPCHECK_EXECUTABLE)
-- cppcheck: The executable was not found
-- Configuring done (0.9s)
-- Generating done (3.8s)
-- Build files have been written to: /Users/myself/src/osquery/build
second one is in progress, taking (much) more time
s
Yeah; incremental build and from scratch rebuild is slower with `make`;
ninja
is much faster, although a scratch build may be slower due to
openssl
build system not being parallelized in that case.
g
52% and counting.... 😱
s
hum, this suggests me not everything was actually built then. Shouldn't take this much otherwise. You should actually see .c or .cpp files being mentioned vs just target names if it's building.
g
yeah, I'm kinda very surprised that it takes so much time 🤔 . It usually doesn't.
still not finished, but at least
test_enroll_secret.txt
is now present in the directory
s
One thing I would be careful about is memory usage, If you have a machine with a lot of cores but not much ram. It roughly takes 2GB per core when also building tests, so maybe some jobs may have crashed.
g
this one has 64GB of ram, and "only" 10 cores... so that is not likely to be the issue, but I can reduce the number of threads, just in case
s
Well you would notice on the result of the build. But that amount sounds good!
g
full build is done, do you want to see the output ?
s
If the build succeeded and since you saw that file appearing, my guess is that things now will work. Keep it on the side at least, and re-run the tests, we'll go from there.
g
Copy code
13/81 Test #13: osquery_distributed_tests-test ........................................   Passed    1.45 sec
looks better than last time !
Copy code
100% tests passed, 0 tests failed out of 81

Total Test time (real) = 947.55 sec
yay ! Thanks a lot for your help @Stefano Bonicatti! I can now move forward on my PR