Fix OSS CI test and link coverage

Summary: Ensure OSS CI exercises a nonempty, backend-free Python test inventory instead of reporting a successful run with zero tests. Also repair the legacy Automake fbthrift static-link closure so external builds can link the mock server reliably.

Reviewed By: anidev

Differential Revision: D113608290

fbshipit-source-id: 5cd265c04f72bc37dfbc919511d2892f04d0cbf4

Author: John Hugg

CMake/mcrouter_test_config.py.in

@@ -0,0 +1,34 @@
+#!/usr/bin/env python3
+# Copyright (c) Meta Platforms, Inc. and affiliates.
+#
+# This source code is licensed under the MIT license found in the
+# LICENSE file in the root directory of this source tree.
+
+
+class ThriftTestClient:
+    def __init__(self, addr, port) -> None:
+        raise NotImplementedError
+
+    def sendVersion(self) -> str:
+        raise NotImplementedError
+
+
+class McrouterGlobals:
+    @staticmethod
+    def binPath(name):
+        bins = {
+            "mcrouter": "@CMAKE_CURRENT_BINARY_DIR@/bin/mcrouter",
+        }
+        return bins[name]
+
+    @staticmethod
+    def preprocessArgs(args):
+        return args
+
+    @staticmethod
+    def useThriftClient():
+        return False
+
+    @staticmethod
+    def createThriftTestClient(addr, port) -> ThriftTestClient:
+        raise NotImplementedError

CMakeLists.txt

@@ -573,9 +573,90 @@ install(
 # =============================================================================
 if(BUILD_TESTS)
   enable_testing()
-  find_package(GTest CONFIG REQUIRED)
+  include(FBPythonBinary)
+
+  set(MCROUTER_TEST_CONFIG_DIR "${CMAKE_CURRENT_BINARY_DIR}/python")
+  file(MAKE_DIRECTORY "${MCROUTER_TEST_CONFIG_DIR}")
+  configure_file(
+    "${CMAKE_CURRENT_SOURCE_DIR}/CMake/mcrouter_test_config.py.in"
+    "${MCROUTER_TEST_CONFIG_DIR}/config.py"
+    @ONLY
+  )
+
+  add_fb_python_library(
+    mcrouter_test_config
+    BASE_DIR "${MCROUTER_TEST_CONFIG_DIR}"
+    NAMESPACE "mcrouter.test"
+    SOURCES "${MCROUTER_TEST_CONFIG_DIR}/config.py"
+  )
+
+  # JSON fixtures required by test_validate_config.py are referenced via
+  # relative path "mcrouter/test/*.json" with WORKING_DIRECTORY set below.
+  # In fbcode layout canonical JSONs live at fbcode/mcrouter/test/ (../test
+  # from public_tld); in OSS after shipit they live at mcrouter/test/ in the
+  # source tree. We copy them to binary dir only (no source-tree mutation)
+  # and run tests with WORKING_DIRECTORY = binary dir so
+  # "mcrouter/test/*.json" resolves in sandboxed/OSS executions.
+  file(MAKE_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/mcrouter/test")
+  # fbcode layout: fbcode/mcrouter/test/ (../test from public_tld).
+  set(_fbcode_test_dir "${CMAKE_CURRENT_SOURCE_DIR}/../test")
+  # OSS layout (after shipit): fixtures vendored under mcrouter/test/.
+  set(_oss_test_dir "${CMAKE_CURRENT_SOURCE_DIR}/mcrouter/test")
+  if(EXISTS "${_fbcode_test_dir}/test_ascii.json"
+     AND EXISTS "${_fbcode_test_dir}/invalid_config.json")
+    file(
+      COPY
+        "${_fbcode_test_dir}/test_ascii.json"
+        "${_fbcode_test_dir}/invalid_config.json"
+      DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/mcrouter/test"
+    )
+  elseif(EXISTS "${_oss_test_dir}/test_ascii.json"
+         AND EXISTS "${_oss_test_dir}/invalid_config.json")
+    file(
+      COPY
+        "${_oss_test_dir}/test_ascii.json"
+        "${_oss_test_dir}/invalid_config.json"
+      DESTINATION "${CMAKE_CURRENT_BINARY_DIR}/mcrouter/test"
+    )
+  else()
+    message(WARNING
+      "mcrouter test JSON fixtures (test_ascii.json, invalid_config.json) were "
+      "not found under ../test or mcrouter/test. test_validate_config.py will "
+      "fail with FileNotFoundError at runtime. Only test_validate_config.py "
+      "depends on these fixtures; test_bad_params.py and test_linenumbers.py "
+      "do not."
+    )
+  endif()
+
+  add_fb_python_unittest(
+    mcrouter_oss_python_tests
+    BASE_DIR "${CMAKE_CURRENT_SOURCE_DIR}"
+    NAMESPACE ""
+    WORKING_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}"
+    SOURCES
+      mcrouter/test/test_bad_params.py
+      mcrouter/test/test_linenumbers.py
+      mcrouter/test/test_validate_config.py
+    DEPENDS mcrouter_test_config
+  )
 
-  # Add test subdirectories when ready
-  # add_subdirectory(mcrouter/lib/test)
-  # add_subdirectory(mcrouter/test)
+  # Ensure mcrouter binary exists before Python tests that spawn it.
+  # We avoid depending on the internal "${TARGET}.GEN_PY_EXE" name from
+  # FBPythonBinary.cmake – it is an implementation detail that can change
+  # across fbcode_builder revisions. Instead, make the
+  # mcrouter_test_config.py_sources_built target (which add_fb_python_library
+  # creates and which chains into the unittest exe build) depend on
+  # mcrouter_server. That way ctest only runs after the server binary is
+  # present. This target name is still an FBPythonBinary internal, so guard
+  # on its existence too and fall back to a warning if the contract changes.
+  if(TARGET mcrouter_server AND TARGET mcrouter_test_config.py_sources_built)
+    add_dependencies(mcrouter_test_config.py_sources_built mcrouter_server)
+  elseif(TARGET mcrouter_server)
+    message(WARNING
+      "Expected FBPythonBinary target mcrouter_test_config.py_sources_built "
+      "was not found; cannot order the mcrouter_server build before the "
+      "Python tests. Tests that spawn mcrouter may race the binary build if "
+      "FBPythonBinary's internal target naming has changed."
+    )
+  endif()
 endif()

mcrouter/lib/network/test/Makefile.am

@@ -7,29 +7,36 @@ noinst_PROGRAMS = mock_mc_server
 noinst_LIBRARIES = libtest_util.a
 check_PROGRAMS = mcrouter_network_test
 
-mock_mc_server_SOURCES = \
-  MockMc.cpp \
-  MockMc.h \
-  MockMcOnRequest.h \
-  MockMcServer.cpp
-mock_mc_server_CPPFLAGS = -I$(top_srcdir)/..
-# Link order matters.
-mock_mc_server_LDADD = \
-	$(top_builddir)/lib/libmcrouter.a \
+FBTHRIFT_STATIC_LIBS = \
+	-Wl,--start-group \
 	-lthriftcpp2 \
 	-lserverdbginfo \
 	-ltransport \
 	-lthriftanyrep \
+	-lthriftannotation \
 	-lthrifttype \
 	-lthrifttyperep \
-	-lthriftprotocol \
+	-lthrift_dynamic_value \
 	-lrpcmetadata \
+	-lthriftprotocol \
 	-lthriftmetadata \
 	-lasync \
 	-lconcurrency \
 	-lruntime \
 	-lthrift-core \
 	-lcommon \
+	-Wl,--end-group
+
+mock_mc_server_SOURCES = \
+  MockMc.cpp \
+  MockMc.h \
+  MockMcOnRequest.h \
+  MockMcServer.cpp
+mock_mc_server_CPPFLAGS = -I$(top_srcdir)/..
+# Link order matters.
+mock_mc_server_LDADD = \
+	$(top_builddir)/lib/libmcrouter.a \
+	$(FBTHRIFT_STATIC_LIBS) \
 	-lfmt \
 	-lfizz \
 	-lwangle \
@@ -44,14 +51,7 @@ mock_mc_thrift_server_CPPFLAGS = -I$(top_srcdir)/..
 # Link order matters.
 mock_mc_thrift_server_LDADD = \
 	$(top_builddir)/lib/libmcrouter.a \
-	-lthriftcpp2 \
-	-ltransport \
-	-lthriftprotocol \
-	-lrpcmetadata \
-	-lasync \
-	-lconcurrency \
-	-lthrift-core \
-	-lcommon \
+	$(FBTHRIFT_STATIC_LIBS) \
 	-lfmt \
 	-lfizz \
 	-lwangle \
@@ -66,14 +66,11 @@ mock_mc_server_dual_CPPFLAGS = -I$(top_srcdir)/..
 # Link order matters.
 mock_mc_server_dual_LDADD = \
 	$(top_builddir)/lib/libmcrouter.a \
-	-lthriftcpp2 \
-	-ltransport \
-	-lthriftprotocol \
-	-lrpcmetadata \
-	-lasync \
-	-lconcurrency \
-	-lthrift-core \
-	-lcommon
+	$(FBTHRIFT_STATIC_LIBS) \
+	-lfmt \
+	-lfizz \
+	-lwangle \
+	-lfolly
 
 libtest_util_a_SOURCES = \
   ClientSocket.cpp \
@@ -110,8 +107,4 @@ mcrouter_network_test_LDADD = \
   $(top_builddir)/lib/libmcrouter.a \
   $(top_builddir)/lib/libtestmain.la \
   $(top_builddir)/lib/network/libtest_util.a \
-  -lthriftcpp2 \
-  -lthriftprotocol \
-  -lrpcmetadata \
-  -ltransport \
-  -lcommon
+  $(FBTHRIFT_STATIC_LIBS)