LeelaChessZero / lc0

The rewritten engine, originally for tensorflow. Now all other backends have been ported here.
GNU General Public License v3.0
2.38k stars 525 forks source link

optionsparser_test fails on Mac #1439

Open gsobala opened 3 years ago

gsobala commented 3 years ago

(a) works fine on Ubuntu and Windows (b) lc0 command-line options work just fine on Mac despite this test fail. All other tests pass. (c) if we don't really use gtest anyway (how long has this been unnoticed?) then perhaps we should set the meson compile option default to false on all systems. It is already false by default on Windows.

BUG REPORT

optionsparser_test fails on Mac

A clear and concise description of what the bug is.

  1. Compile and run optionsparser_test on Mac OSX 10.15.6 using default settings

Expected behavior: Test should pass Observed behavior:

% ./optionsparser_test
[==========] Running 5 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 5 tests from OptionsParser
[ RUN      ] OptionsParser.CheckInvalidOption
../../src/utils/optionsparser_test.cc:33: Failure
Expected: options.SetUciOption("this-is-an-invalid-option", "0") throws an exception of type Exception.
  Actual: it throws a different type.
[  FAILED  ] OptionsParser.CheckInvalidOption (0 ms)
[ RUN      ] OptionsParser.IntOptionCheckValueConstraints
../../src/utils/optionsparser_test.cc:44: Failure
Expected: options.SetUciOption("int-test-a", "0") throws an exception of type Exception.
  Actual: it throws a different type.
../../src/utils/optionsparser_test.cc:45: Failure
Expected: options.SetUciOption("int-test-a", "100") throws an exception of type Exception.
  Actual: it throws a different type.
[  FAILED  ] OptionsParser.IntOptionCheckValueConstraints (0 ms)
[ RUN      ] OptionsParser.FloatOptionCheckValueConstraints
../../src/utils/optionsparser_test.cc:56: Failure
Expected: options.SetUciOption("float-test-a", "0.0") throws an exception of type Exception.
  Actual: it throws a different type.
../../src/utils/optionsparser_test.cc:57: Failure
Expected: options.SetUciOption("float-test-a", "100.0") throws an exception of type Exception.
  Actual: it throws a different type.
[  FAILED  ] OptionsParser.FloatOptionCheckValueConstraints (0 ms)
[ RUN      ] OptionsParser.BoolOptionsCheckValueConstraints
../../src/utils/optionsparser_test.cc:67: Failure
Expected: options.SetUciOption("bool-test-a", "leela") throws an exception of type Exception.
  Actual: it throws a different type.
[  FAILED  ] OptionsParser.BoolOptionsCheckValueConstraints (0 ms)
[ RUN      ] OptionsParser.ChoiceOptionCheckValueConstraints
../../src/utils/optionsparser_test.cc:82: Failure
Expected: options.SetUciOption("choice-test-a", "choice-d") throws an exception of type Exception.
  Actual: it throws a different type.
[  FAILED  ] OptionsParser.ChoiceOptionCheckValueConstraints (0 ms)
[----------] 5 tests from OptionsParser (1 ms total)

[----------] Global test environment tear-down
[==========] 5 tests from 1 test suite ran. (1 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 5 tests, listed below:
[  FAILED  ] OptionsParser.CheckInvalidOption
[  FAILED  ] OptionsParser.IntOptionCheckValueConstraints
[  FAILED  ] OptionsParser.FloatOptionCheckValueConstraints
[  FAILED  ] OptionsParser.BoolOptionsCheckValueConstraints
[  FAILED  ] OptionsParser.ChoiceOptionCheckValueConstraints

Lc0 version lc0 master d9c90c8fbd8c22875dda94e475ba18c8f37aeed3

borg323 commented 3 years ago

Looking at the code, all instances of gtest's EXPECT_THROW fail here and the code explicitly throws exceptions of type Exception. This looks like a gtest build issue, is this with the meson subproject or with a pre-built library?

gsobala commented 3 years ago

meson subproject ... the following is from meson-log.txt :

Running compile:
Working directory:  /Users/george/Development/lc0/build/release/meson-private/tmp3xl7cnt4
Command line:  c++ /Users/george/Development/lc0/build/release/meson-private/tmp3xl7cnt4/testfile.cpp -o /Users/george/Development/lc0/build/release/meson-private/tmp3xl7cnt4/output.exe -pipe -O0 -fpermissive -lgtest -Wl,-undefined,dynamic_lookup 

Code:
 int main(void) { return 0; }

Compiler stdout:

Compiler stderr:
 ld: library not found for -lgtest
clang: error: linker command failed with exit code 1 (use -v to see invocation)

None of 'CXX_LD' are defined in the environment, not changing global flags.
Running compile:
Working directory:  /Users/george/Development/lc0/build/release/meson-private/tmpjrj7zwrp
Command line:  c++ /Users/george/Development/lc0/build/release/meson-private/tmpjrj7zwrp/testfile.cpp -o /Users/george/Development/lc0/build/release/meson-private/tmpjrj7zwrp/output.exe -pipe -O0 -fpermissive -lgtest_main -Wl,-undefined,dynamic_lookup 

Code:
 int main(void) { return 0; }

Compiler stdout:

Compiler stderr:
 ld: library not found for -lgtest_main
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Run-time dependency GTest found: NO (tried pkgconfig and system)
Looking for a fallback subproject for the dependency gtest

Executing subproject gtest method meson 

None of 'PKG_CONFIG_PATH' are defined in the environment, not changing global flags.
None of 'PKG_CONFIG_PATH' are defined in the environment, not changing global flags.
Project name: gtest
Project version: 1.10.0
C++ compiler for the build machine: c++ (clang 11.0.3 "Apple clang version 11.0.3 (clang-1103.0.32.62)")
C++ linker for the build machine: c++ ld64 556.6
C++ compiler for the host machine: c++ (clang 11.0.3 "Apple clang version 11.0.3 (clang-1103.0.32.62)")
C++ linker for the host machine: c++ ld64 556.6
Dependency threads found: YES unknown (cached)
Dependency threads found: YES unknown (cached)
Dependency threads found: YES unknown (cached)
Dependency threads found: YES unknown (cached)
Build targets in project: 1
Subproject gtest finished.

Dependency gtest from subproject subprojects/gtest found: YES 1.10.0
Adding test "ChessBoard"
Adding test "HashCat"
Adding test "PositionTest"
Adding test "OptionsParserTest"
Adding test "SyzygyTest"
Adding test "EncodePositionForNN"
Build targets in project: 8

Fails on all three of my iMac Pro, a fresh install of Catalina in a VM, and in appveyor, so not specific to some quirk of my system.

gsobala commented 3 years ago

Changing all instances of Exception to explicit type std::runtime_error in optionsparser_test.cc fixes this. Clearly this should not be required, but Apple.

george@Georges-iMac-Pro-2 lc0 % git diff master
diff --git a/src/utils/optionsparser_test.cc b/src/utils/optionsparser_test.cc
index 1a620eb..7dfa60c 100644
--- a/src/utils/optionsparser_test.cc
+++ b/src/utils/optionsparser_test.cc
@@ -30,7 +30,7 @@ TEST(OptionsParser, CheckInvalidOption) {
   EXPECT_NO_THROW(
       options.SetUciOption("this-is-a-valid-option", "valid-value"));
   EXPECT_THROW(options.SetUciOption("this-is-an-invalid-option", "0"),
-               Exception);
+               std::runtime_error);
 }

 TEST(OptionsParser, IntOptionCheckValueConstraints) {
@@ -41,8 +41,8 @@ TEST(OptionsParser, IntOptionCheckValueConstraints) {
   EXPECT_NO_THROW(options.SetUciOption("int-test-a", "25"));
   EXPECT_NO_THROW(options.SetUciOption("int-test-a", "50"));
   EXPECT_NO_THROW(options.SetUciOption("int-test-a", "75"));
-  EXPECT_THROW(options.SetUciOption("int-test-a", "0"), Exception);
-  EXPECT_THROW(options.SetUciOption("int-test-a", "100"), Exception);
+  EXPECT_THROW(options.SetUciOption("int-test-a", "0"), std::runtime_error);
+  EXPECT_THROW(options.SetUciOption("int-test-a", "100"), std::runtime_error);
 }

 TEST(OptionsParser, FloatOptionCheckValueConstraints) {
@@ -53,8 +53,8 @@ TEST(OptionsParser, FloatOptionCheckValueConstraints) {
   EXPECT_NO_THROW(options.SetUciOption("float-test-a", "25.0"));
   EXPECT_NO_THROW(options.SetUciOption("float-test-a", "50.0"));
   EXPECT_NO_THROW(options.SetUciOption("float-test-a", "75.0"));
-  EXPECT_THROW(options.SetUciOption("float-test-a", "0.0"), Exception);
-  EXPECT_THROW(options.SetUciOption("float-test-a", "100.0"), Exception);
+  EXPECT_THROW(options.SetUciOption("float-test-a", "0.0"), std::runtime_error);
+  EXPECT_THROW(options.SetUciOption("float-test-a", "100.0"), std::runtime_error);
 }

 TEST(OptionsParser, BoolOptionsCheckValueConstraints) {
@@ -64,7 +64,7 @@ TEST(OptionsParser, BoolOptionsCheckValueConstraints) {

   EXPECT_NO_THROW(options.SetUciOption("bool-test-a", "true"));
   EXPECT_NO_THROW(options.SetUciOption("bool-test-a", "false"));
-  EXPECT_THROW(options.SetUciOption("bool-test-a", "leela"), Exception);
+  EXPECT_THROW(options.SetUciOption("bool-test-a", "leela"), std::runtime_error);
 }

 TEST(OptionsParser, ChoiceOptionCheckValueConstraints) {
@@ -79,7 +79,7 @@ TEST(OptionsParser, ChoiceOptionCheckValueConstraints) {
   EXPECT_NO_THROW(options.SetUciOption("choice-test-a", "choice-a"));
   EXPECT_NO_THROW(options.SetUciOption("choice-test-a", "choice-b"));
   EXPECT_NO_THROW(options.SetUciOption("choice-test-a", "choice-c"));
-  EXPECT_THROW(options.SetUciOption("choice-test-a", "choice-d"), Exception);
+  EXPECT_THROW(options.SetUciOption("choice-test-a", "choice-d"), std::runtime_error);
 }

 }  // namespace lczero

Incidentally the latest master of Googletest still fails with apple-clang with the original code but now with more explicit reporting e.g.

Expected: options.SetUciOption("this-is-an-invalid-option", "0") throws an exception of type Exception.
  Actual: it throws lczero::Exception with description "Unknown option: this-is-an-invalid-option".

Changing optionsparser-test.cc to use lczero::Exception rather than Exception just leads to

Expected: options.SetUciOption("this-is-an-invalid-option", "0") throws an exception of type lczero::Exception.
  Actual: it throws lczero::Exception with description "Unknown option: this-is-an-invalid-option".

so there you go. Blame Apple.