Icinga / icinga2

The core of our monitoring platform with a powerful configuration language and REST API.
https://icinga.com/docs/icinga2/latest
GNU General Public License v2.0
2k stars 574 forks source link

GHA: Unbreak Windows Tests #10104

Closed oxzi closed 2 months ago

oxzi commented 2 months ago

As seen in the recent GHA run for #10102, the two Windows Actions have failed. The output log contains:

DEBUG: 27+ >>>> ctest.exe -C "${env:CMAKE_BUILD_TYPE}" -T test -O $env:ICINGA2_BUILDPATH/Test.xml --output-on-failure --log_level=all CMake Error: Unknown argument: --log_level=all CMake Error: Run 'ctest --help' for all supported options.

After consulting ctest(1), older versions included, I have never found a mention of the "--log_level" flag. Since the useful "--output-on-failure" flag is already set, which will "[o]utput anything outputted by the test program if the test should fail", I do not see any further reason for more logging information.

This flag was introduced in 7665143afa500dd589546665124293b9c1206265, but I have not found any reasoning for the flag in particular.

julianbrost commented 2 months ago

After consulting ctest(1), older versions included, I have never found a mention of the "--log_level" flag.

That seems to be a parameter for Boost.Test, but I don't know if that's even the right place to put it (if it fails with "Unknown argument", it probably isn't).

oxzi commented 2 months ago

Seems to work! Both Windows jobs are all green now. The failing centos job was just removed in #10095.

julianbrost commented 2 months ago

Let's find out what the CI does.

That would also be interesting for a failing test, so would you mind breaking a test intentionally to see how the output will look like in GitHub Actions? Also a good opportunity to get rid of the CentOS job :sweat_smile:

yhabteab commented 2 months ago

Let's find out what the CI does.

That would also be interesting for a failing test, so would you mind breaking a test intentionally to see how the output will look like in GitHub Actions? Also a good opportunity to get rid of the CentOS job 😅

Nonetheless, I would set this option via the Boost_TEST_FLAGS variable (-DBoost_TEST_FLAGS='--catch_system_error=yes --log_level=all'). Btw. your recent pushes don't seem to break the tests on windows.

oxzi commented 2 months ago

Btw. your recent pushes don't seem to break the tests on windows.

Maybe because the test is broken? I have switched the logic in the host_flapping test.. but it passed anyway.

161/163 Test #161: icinga_checkable-icinga_checkable_flapping/host_flapping ................ Passed 0.02 sec

Now, I have additionally changed something in the array check which should really fail.

yhabteab commented 2 months ago

Nonetheless, I would set this option via the Boost_TEST_FLAGS variable (-DBoost_TEST_FLAGS='--catch_system_error=yes --log_level=all').

NOPE! That doesn't seem to be necessary at all, so you can ignore that!

julianbrost commented 2 months ago

Btw. your recent pushes don't seem to break the tests on windows.

Maybe because the test is broken? I have switched the logic in the host_flapping test.. but it passed anyway.

I guess it's because of that: https://github.com/Icinga/icinga2/blob/2ea174a0f4dcb30a0e0a25ea6e540670460a343f/test/icinga-checkable-flapping.cpp#L94-L95

oxzi commented 2 months ago

It breaks at last.

Details w/ Logs

On Windows: ``` Test project D:/a/icinga2/icinga2/windows-icinga2/icinga2/Build Start 1: base-base_array/construct 1/163 Test #1: base-base_array/construct ............................................... Passed 0.57 sec Start 2: base-base_array/getset 2/163 Test #2: base-base_array/getset ..................................................***Failed 0.03 sec Running 1 test case... D:/a/icinga2/icinga2/windows-icinga2/icinga2/test/base-array.cpp(25): error: in "base_array/getset": check array->GetLength() == 23 has failed D:/a/icinga2/icinga2/windows-icinga2/icinga2/test/base-array.cpp(26): error: in "base_array/getset": check array->Get(0) == 42 has failed D:/a/icinga2/icinga2/windows-icinga2/icinga2/test/base-array.cpp(27): error: in "base_array/getset": check array->Get(1) == 23 has failed D:/a/icinga2/icinga2/windows-icinga2/icinga2/test/base-array.cpp(28): error: in "base_array/getset": check array->Get(2) == 42 has failed *** 4 failures are detected in the test module "icinga2" [ . . . ] 99% tests passed, 1 tests failed out of 163 Total Test time (real) = 60.52 sec The following tests FAILED: 2 - base-base_array/getset (Failed) Errors while running CTest ``` And on Debian: ``` [0/1] Running tests... Test project /icinga2/build Start 1: base-base_array/construct 1/165 Test #1: base-base_array/construct ............................................... Passed 0.01 sec Start 2: base-base_array/getset 2/165 Test #2: base-base_array/getset ..................................................***Failed 0.01 sec Running 1 test case... /icinga2/test/base-array.cpp(25): error: in "base_array/getset": check array->GetLength() == 23 has failed /icinga2/test/base-array.cpp(26): error: in "base_array/getset": check array->Get(0) == 42 has failed /icinga2/test/base-array.cpp(27): error: in "base_array/getset": check array->Get(1) == 23 has failed /icinga2/test/base-array.cpp(28): error: in "base_array/getset": check array->Get(2) == 42 has failed *** 4 failures are detected in the test module "icinga2" [ . . . ] 99% tests passed, 1 tests failed out of 165 Total Test time (real) = 33.83 sec Errors while running CTest ```

Thus, I would say both the behavior and output looks identical. I am going to remove the breaking commit, than it should be fine?