From 15bf44da977bbd7b36161a0f7b48f56765befa0d Mon Sep 17 00:00:00 2001 From: Bart Louwers Date: Sat, 5 Aug 2023 09:48:15 +0200 Subject: [PATCH] Add expression test runner to Bazel (#1464) --- .github/workflows/linux-ci.yml | 4 ++-- expression-test/BUILD.bazel | 19 ++++++++++++++++ expression-test/CMakeLists.txt | 6 ----- expression-test/expression_test_logger.cpp | 4 ++-- expression-test/expression_test_parser.cpp | 25 +++++++++++---------- expression-test/expression_test_parser.hpp | 9 ++++---- expression-test/expression_test_runner.cpp | 4 ++-- expression-test/filesystem.hpp | 9 -------- expression-test/main.cpp | 9 ++++++-- metrics/BUILD.bazel | 15 +++++++++++++ platform/linux/src/headless_backend_glx.cpp | 6 +++-- test/BUILD.bazel | 1 - 12 files changed, 68 insertions(+), 43 deletions(-) delete mode 100644 expression-test/filesystem.hpp diff --git a/.github/workflows/linux-ci.yml b/.github/workflows/linux-ci.yml index ccd66aed2..dd36af024 100644 --- a/.github/workflows/linux-ci.yml +++ b/.github/workflows/linux-ci.yml @@ -213,8 +213,8 @@ jobs: xvfb-run -a \ bazel coverage --combined_report=lcov --instrumentation_filter="//:mbgl-core" --//:maplibre_platform=linux \ --test_output=errors --local_test_jobs=1 \ - --test_env=DISPLAY --test_env=XAUTHORITY --define=CI_BUILD=1 \ - //test:core + --test_env=DISPLAY --test_env=XAUTHORITY --copt="-DCI_BUILD" \ + //test:core //expression-test:test - name: Upload coverage report if: '!cancelled()' diff --git a/expression-test/BUILD.bazel b/expression-test/BUILD.bazel index 4f0f2e798..2d5b7daca 100644 --- a/expression-test/BUILD.bazel +++ b/expression-test/BUILD.bazel @@ -18,3 +18,22 @@ cc_library( "//:mbgl-core", ], ) + +cc_test( + name = "test", + size = "small", + srcs = glob( + [ + "*.cpp", + "*.hpp", + ], + allow_empty = False, + ), + copts = CPP_FLAGS + MAPLIBRE_FLAGS, + data = [ + "//metrics:expression-test-files", + ], + deps = [ + "//platform/linux:impl", + ], +) diff --git a/expression-test/CMakeLists.txt b/expression-test/CMakeLists.txt index c6a2f5b91..f49277034 100644 --- a/expression-test/CMakeLists.txt +++ b/expression-test/CMakeLists.txt @@ -11,11 +11,6 @@ add_executable( ${PROJECT_SOURCE_DIR}/expression-test/main.cpp ) -target_compile_definitions( - mbgl-expression-test - PRIVATE TEST_RUNNER_ROOT_PATH="${PROJECT_SOURCE_DIR}" -) - # FIXME: Should not use core private interface target_include_directories( mbgl-expression-test @@ -27,7 +22,6 @@ target_link_libraries( PRIVATE Mapbox::Base Mapbox::Base::Extras::args - Mapbox::Base::Extras::filesystem mbgl-core mbgl-compiler-options ) diff --git a/expression-test/expression_test_logger.cpp b/expression-test/expression_test_logger.cpp index a6d513466..1f7aa6187 100644 --- a/expression-test/expression_test_logger.cpp +++ b/expression-test/expression_test_logger.cpp @@ -1,11 +1,11 @@ #include "expression_test_logger.hpp" #include "expression_test_runner.hpp" -#include "filesystem.hpp" #include #include #include +#include using namespace mbgl; using namespace std::literals; @@ -172,7 +172,7 @@ void printStats(const TestStats& stats) { } void writeHTMLResults(const TestStats& stats, const std::string& rootPath, bool shuffle, uint32_t seed) { - filesystem::path path = filesystem::path(rootPath) / "index.html"s; + std::filesystem::path path = std::filesystem::path(rootPath) / "index.html"s; try { util::write_file(path.string(), createResultPage(stats, shuffle, seed)); printf("Results at: %s\n", path.string().c_str()); diff --git a/expression-test/expression_test_parser.cpp b/expression-test/expression_test_parser.cpp index 464292a3f..a8fc482e8 100644 --- a/expression-test/expression_test_parser.cpp +++ b/expression-test/expression_test_parser.cpp @@ -53,8 +53,8 @@ void writeJSON(rapidjson::PrettyWriter& writer, const V using ErrorMessage = std::string; using JSONReply = variant; -JSONReply readJson(const filesystem::path& jsonPath) { - auto maybeJSON = util::readFile(jsonPath); +JSONReply readJson(const std::filesystem::path& jsonPath) { + auto maybeJSON = util::readFile(jsonPath.string()); if (!maybeJSON) { return {"Unable to open file "s + jsonPath.string()}; } @@ -309,8 +309,9 @@ bool parseInputs(const JSValue& inputsValue, TestData& data) { } // namespace -std::tuple, bool, uint32_t> parseArguments(int argc, char** argv) { - args::ArgumentParser argumentParser("Mapbox GL Expression Test Runner"); +std::tuple, bool, uint32_t> parseArguments(int argc, + char** argv) { + args::ArgumentParser argumentParser("MapLibre Native Expression Test Runner"); args::HelpFlag helpFlag(argumentParser, "help", "Display this help menu", {'h', "help"}); args::Flag shuffleFlag(argumentParser, "shuffle", "Toggle shuffling the tests order", {'s', "shuffle"}); @@ -339,13 +340,13 @@ std::tuple, bool, uint32_t> pars exit(2); } - filesystem::path rootPath{std::string(TEST_RUNNER_ROOT_PATH).append("/metrics/integration/expression-tests")}; - if (!filesystem::exists(rootPath)) { + const auto rootPath = std::filesystem::current_path().append("metrics/integration/expression-tests"); + if (!std::filesystem::exists(rootPath)) { Log::Error(Event::General, "Test path '" + rootPath.string() + "' does not exist."); exit(3); } - std::vector paths; + std::vector paths; for (const auto& testName : args::get(testNameValues)) { paths.emplace_back(rootPath.string() + "/" + testName); } @@ -356,15 +357,15 @@ std::tuple, bool, uint32_t> pars auto testFilter = testFilterValue ? args::get(testFilterValue) : std::string{}; // Recursively traverse through the test paths and collect test directories containing "test.json". - std::vector testPaths; + std::vector testPaths; testPaths.reserve(paths.size()); for (const auto& path : paths) { - if (!filesystem::exists(path)) { + if (!std::filesystem::exists(path)) { Log::Warning(Event::General, "Provided test folder '" + path.string() + "' does not exist."); continue; } - for (auto& testPath : filesystem::recursive_directory_iterator(path)) { + for (auto& testPath : std::filesystem::recursive_directory_iterator(path)) { if (!testFilter.empty() && !std::regex_search(testPath.path().string(), std::regex(testFilter))) { continue; } @@ -382,7 +383,7 @@ std::tuple, bool, uint32_t> pars Ignores parseExpressionIgnores() { Ignores ignores; - const auto mainIgnoresPath = filesystem::path(TEST_RUNNER_ROOT_PATH).append("metrics/ignores/platform-all.json"); + const auto mainIgnoresPath = std::filesystem::current_path().append("metrics/ignores/platform-all.json"); auto maybeIgnores = readJson(mainIgnoresPath); if (!maybeIgnores.is()) { // NOLINT return {}; @@ -401,7 +402,7 @@ Ignores parseExpressionIgnores() { return ignores; } -std::optional parseTestData(const filesystem::path& path) { +std::optional parseTestData(const std::filesystem::path& path) { try { TestData data; auto maybeJson = readJson(path.string()); diff --git a/expression-test/expression_test_parser.hpp b/expression-test/expression_test_parser.hpp index b1d7b4772..3cfe24896 100644 --- a/expression-test/expression_test_parser.hpp +++ b/expression-test/expression_test_parser.hpp @@ -1,7 +1,5 @@ #pragma once -#include "filesystem.hpp" - #include #include #include @@ -9,6 +7,7 @@ #include #include #include +#include using namespace mbgl; @@ -79,12 +78,12 @@ struct Ignore { std::string reason; }; -using Arguments = std::tuple, bool, uint32_t>; +using Arguments = std::tuple, bool, uint32_t>; Arguments parseArguments(int argc, char** argv); using Ignores = std::vector; Ignores parseExpressionIgnores(); -std::optional parseTestData(const filesystem::path&); +std::optional parseTestData(const std::filesystem::path&); std::string toJSON(const Value& value, unsigned indent = 0, bool singleLine = false); JSDocument toDocument(const Value&); @@ -96,4 +95,4 @@ std::unique_ptr parseExpression(const JSValue&, TestResult&); std::unique_ptr parseExpression(const std::optional&, std::optional&, - TestResult&); + TestResult&); \ No newline at end of file diff --git a/expression-test/expression_test_runner.cpp b/expression-test/expression_test_runner.cpp index 3a322a5b2..1987b7026 100644 --- a/expression-test/expression_test_runner.cpp +++ b/expression-test/expression_test_runner.cpp @@ -1,7 +1,6 @@ #include "expression_test_runner.hpp" #include "expression_test_logger.hpp" #include "expression_test_parser.hpp" -#include "filesystem.hpp" #include "test_runner_common.hpp" #include @@ -12,6 +11,7 @@ #include #include +#include using namespace std::literals; @@ -87,7 +87,7 @@ void writeTestData(const JSDocument& document, const std::string& rootPath, cons writer.SetIndent(' ', 2); document.Accept(writer); buffer.Put('\n'); - filesystem::path path = filesystem::path(rootPath) / id / "test.json"s; + std::filesystem::path path = std::filesystem::path(rootPath) / id / "test.json"s; try { util::write_file(path.string(), {buffer.GetString(), buffer.GetSize()}); } catch (std::exception&) { diff --git a/expression-test/filesystem.hpp b/expression-test/filesystem.hpp deleted file mode 100644 index cee7e9d91..000000000 --- a/expression-test/filesystem.hpp +++ /dev/null @@ -1,9 +0,0 @@ -#pragma once - -#include - -namespace mbgl { - -namespace filesystem = ghc::filesystem; - -} // namespace mbgl diff --git a/expression-test/main.cpp b/expression-test/main.cpp index e81c26168..4c1fd32ba 100644 --- a/expression-test/main.cpp +++ b/expression-test/main.cpp @@ -2,13 +2,18 @@ #include "expression_test_runner.hpp" #include "expression_test_logger.hpp" +#include + #include #include +#include int main(int argc, char** argv) try { + Log::useLogThread(false); + // Parse args - std::vector testPaths; - mbgl::filesystem::path rootPath; + std::vector testPaths; + std::filesystem::path rootPath; bool shuffle; uint32_t seed; std::tie(rootPath, testPaths, shuffle, seed) = parseArguments(argc, argv); diff --git a/metrics/BUILD.bazel b/metrics/BUILD.bazel index d46427c1d..b0971ef69 100644 --- a/metrics/BUILD.bazel +++ b/metrics/BUILD.bazel @@ -24,3 +24,18 @@ filegroup( "//render-test:__subpackages__", ], ) + +filegroup( + name = "expression-test-files", + srcs = glob( + [ + "integration/expression-tests/**", + ], + allow_empty = False, + ) + [ + "ignores/platform-all.json", + ], + visibility = [ + "//expression-test:__pkg__", + ], +) diff --git a/platform/linux/src/headless_backend_glx.cpp b/platform/linux/src/headless_backend_glx.cpp index 9132d35cb..bf13ac81f 100644 --- a/platform/linux/src/headless_backend_glx.cpp +++ b/platform/linux/src/headless_backend_glx.cpp @@ -7,6 +7,7 @@ #ifdef CI_BUILD #include #include +#include #endif #include @@ -31,7 +32,7 @@ public: #ifdef CI_BUILD // XOpenDisplay has proven very unreliable on CI, perform some retries if it fails - constexpr auto CI_RETRIES = 10; + constexpr auto CI_RETRIES = 20; for (int i = 0; i < CI_RETRIES; i++) { #endif xDisplay = XOpenDisplay(nullptr); @@ -39,7 +40,8 @@ public: if (xDisplay != nullptr) { break; } - Log::Debug(Event::OpenGL, "[CI] Failed to open X display, retrying..."); + // std::cerr is used because the tests expect logging to be deterministic + std::cerr << "[CI] Failed to open X display, retrying..." << std::endl; std::this_thread::sleep_for(std::chrono::milliseconds(500)); } #endif diff --git a/test/BUILD.bazel b/test/BUILD.bazel index fcf45bb5e..5c8798780 100644 --- a/test/BUILD.bazel +++ b/test/BUILD.bazel @@ -26,7 +26,6 @@ cc_library( "src/mbgl/test/*.hpp", ]), copts = CPP_FLAGS + MAPLIBRE_FLAGS, - defines = ["CI_BUILD"], strip_include_prefix = "src", deps = [ "testutils",