diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml index 5f54f6581d..f5850d9fd7 100644 --- a/.github/workflows/unit_tests.yml +++ b/.github/workflows/unit_tests.yml @@ -54,8 +54,10 @@ jobs: shell: bash run: | tar -xvf build_tests.tar - # Multi-config generators (Windows/macOS) need a config; Linux is single-config. - scripts/run_unit_tests.sh "${{ inputs.test-dir }}" "${{ runner.os != 'Linux' && 'Release' || '' }}" + # Every platform builds with a multi-config generator (build_linux.sh uses Ninja + # Multi-Config), so ctest needs the config: without it, plain add_test() tests + # lose their labels and report "Not Run". + scripts/run_unit_tests.sh "${{ inputs.test-dir }}" Release - name: Upload Test Logs if: ${{ failure() }} uses: actions/upload-artifact@v7 diff --git a/AGENTS.md b/AGENTS.md index 01402af3eb..3d0a61b303 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -20,9 +20,9 @@ cmake --build . --config %build_type% --target ALL_BUILD -- -m Catch2 framework. Tests in `tests/`; see [tests/AGENTS.md](tests/AGENTS.md) for where a new test belongs and the conventions to follow. ```bash -cd build && ctest --output-on-failure # all tests -ctest --test-dir ./tests/libslic3r # individual suite -ctest --test-dir ./tests/fff_print +cd build && ctest -C Release --output-on-failure # all tests +ctest --test-dir ./tests/libslic3r -C Release # individual suite +ctest --test-dir ./tests/fff_print -C Release ``` ## Documentation diff --git a/scripts/run_unit_tests.sh b/scripts/run_unit_tests.sh index f6a01dd3e0..1d3f0046fd 100755 --- a/scripts/run_unit_tests.sh +++ b/scripts/run_unit_tests.sh @@ -7,8 +7,9 @@ # # Usage: run_unit_tests.sh [TEST_DIR] [BUILD_CONFIG] # TEST_DIR directory containing the built tests (default: build/tests) -# BUILD_CONFIG configuration to run; required for multi-config generators -# (Windows/macOS), harmless/omitted for single-config (Linux). +# BUILD_CONFIG configuration to run; required for multi-config generators, which all +# build scripts use (build_linux.sh too: Ninja Multi-Config). Without it, +# tests registered with plain add_test() lose their labels and report "Not Run". ROOT_DIR="$(dirname "$0")/.." @@ -17,8 +18,9 @@ cd "${ROOT_DIR}" || exit 1 TEST_DIR="${1:-build/tests}" BUILD_CONFIG="${2:-}" -# Run the whole suite, excluding tests tagged [NotWorking]. +# Run the whole suite, excluding tests tagged [NotWorking] and tests labelled RequiresApp, +# which run the built orca-slicer binary that this directory does not contain. # --no-tests=error fails the job if the filter matches nothing (instead of passing green). -args=(--test-dir "${TEST_DIR}" -LE "NotWorking" --no-tests=error --output-junit "$(pwd)/ctest_results.xml" --output-on-failure -j) +args=(--test-dir "${TEST_DIR}" -LE "NotWorking|RequiresApp" --no-tests=error --output-junit "$(pwd)/ctest_results.xml" --output-on-failure -j) [ -n "${BUILD_CONFIG}" ] && args+=(--build-config "${BUILD_CONFIG}") ctest "${args[@]}" diff --git a/src/OrcaSlicer.cpp b/src/OrcaSlicer.cpp index 218070fa61..47b56bc350 100644 --- a/src/OrcaSlicer.cpp +++ b/src/OrcaSlicer.cpp @@ -189,6 +189,9 @@ typedef struct _sliced_info { int wall_loops{0}; std::vector upward_machines; std::vector downward_machines; + // Structured slicing warnings for result.json, and whether --strict was on. + nlohmann::json warnings = nlohmann::json::array(); + bool strict_mode {false}; }sliced_info_t; std::vector g_slicing_warnings; @@ -424,6 +427,21 @@ static PrinterTechnology get_printer_technology(const DynamicConfig &config) return(ret);} #endif +// Records a structured slicing warning so a CI or scripted consumer can branch on +// a stable `class` string instead of matching stderr. Warnings are kept on the +// run's sliced_info and emitted as the top-level "warnings" array of result.json; +// a non-empty array does not by itself mean the run failed. Under --strict a +// NON_CRITICAL warning additionally ends the run non-zero. +// +// result.json is written on Linux only (see the guard in record_exit_reson), so +// neither "warnings" nor "strict_mode" reaches Windows or macOS. +static void cli_record_warning(sliced_info_t &sliced_info, const std::string &cls, + nlohmann::json details = nlohmann::json::object()) +{ + details["class"] = cls; + sliced_info.warnings.push_back(std::move(details)); +} + void record_exit_reson(std::string outputdir, int code, int plate_id, std::string error_message, sliced_info_t& sliced_info, std::map key_values = std::map()) { #if defined(__linux__) || defined(__LINUX__) @@ -462,6 +480,9 @@ void record_exit_reson(std::string outputdir, int code, int plate_id, std::strin for (auto& iter: key_values) j[iter.first] = iter.second; + j["warnings"] = sliced_info.warnings; + j["strict_mode"] = sliced_info.strict_mode; + boost::nowide::ofstream c; c.open(result_file, std::ios::out | std::ios::trunc); c << j.dump(1, '\t') << std::endl; @@ -1381,6 +1402,16 @@ int CLI::run(int argc, char **argv) bool need_skip = (skip_objects.size() > 0)?true:false; long long global_begin_time = 0, global_current_time; sliced_info_t sliced_info; + // Read up front so result.json reports it for early failures too. + sliced_info.strict_mode = m_config.opt_bool("strict"); + // --no-check skips the check behind the only NON_CRITICAL warning --strict acts on + // (support needed but disabled), from the point it appears among the actions. The pair + // would make --strict a no-op or depend on argument order, so refuse it. + if (sliced_info.strict_mode && m_config.opt_bool("no_check")) { + boost::nowide::cerr << "--strict cannot be combined with --no-check" << std::endl; + record_exit_reson(outfile_dir, CLI_INVALID_PARAMS, 0, cli_errors[CLI_INVALID_PARAMS], sliced_info); + flush_and_exit(CLI_INVALID_PARAMS); + } std::map record_key_values; ConfigOptionBool* downward_check_option = m_config.option("downward_check"); @@ -6009,6 +6040,8 @@ int CLI::run(int argc, char **argv) export_3mf_file = m_config.opt_string(opt_key); }else if(opt_key=="no_check"){ no_check = m_config.opt_bool(opt_key); + }else if(opt_key=="strict"){ + //already read into sliced_info at the start of run() //} else if (opt_key == "export_gcode" || opt_key == "export_sla" || opt_key == "slice") { } else if (opt_key == "normative_check") { //already processed before @@ -6717,6 +6750,15 @@ int CLI::run(int argc, char **argv) if (status.warning_level == PrintStateBase::WarningLevel::NON_CRITICAL) { BOOST_LOG_TRIVIAL(warning) << "plate "<< index+1<< ": found NON_CRITICAL slicing warnings: "<tooltip = L("Do not run any validity checks, such as G-code path conflicts check."); def->set_default_value(new ConfigOptionBool(false)); + // --strict turns the non-critical slicing warnings the CLI otherwise only logs into a + // failed run, and records strict_mode in result.json so consumers can tell the modes apart. + def = this->add("strict", coBool); + def->label = L("Strict mode"); + def->tooltip = L("Exit non-zero when slicing raises a non-critical warning that is " + "otherwise only logged, such as a model that needs support while " + "support is disabled. Use this in CI or scripted pipelines that should " + "never ship a subtly broken slice. Each such warning is also listed " + "with a stable class in the `warnings` array of result.json, which is " + "written on Linux only. Cannot be combined with --no-check, which skips " + "the support check."); + def->set_default_value(new ConfigOptionBool(false)); + def = this->add("normative_check", coBool); def->label = L("Normative check"); def->tooltip = L("Check the normative items."); diff --git a/tests/AGENTS.md b/tests/AGENTS.md index c62a1d12ec..50ea40cafc 100644 --- a/tests/AGENTS.md +++ b/tests/AGENTS.md @@ -10,6 +10,7 @@ Rules for writing tests under `tests/`. [CATCH2.md](CATCH2.md) is the Catch2 ref - `libnest2d`: 2D nesting and packing. - `slic3rutils`: the Python plugin system and its slicing-pipeline bindings. - `filament_group`: filament-to-extruder grouping, checked against golden files. +- `cli`: end-to-end runs of the built `orca-slicer` binary, Linux only. These tests carry the `RequiresApp` label, which the CI unit-test job excludes because it receives only `build/tests`; run them with `ctest --test-dir build/tests -C Release -L RequiresApp`. ## Building and running @@ -17,9 +18,9 @@ Tests are off by default, so the build has to be told to include them. - Windows: `build_release_vs.bat tests`, then `ctest --test-dir build/tests -C Release` - macOS: `./build_release_macos.sh -s -a arm64 -T`, which builds and runs them -- Linux: `./build_linux.sh -t`, then `ctest --test-dir build/tests` +- Linux: `./build_linux.sh -t`, then `ctest --test-dir build/tests -C Release` -Rebuild a single suite with `cmake --build build --config Release --target _tests`. Visual Studio and Xcode are multi-configuration generators, so `ctest` needs `-C` there; on Linux it does not. +Rebuild a single suite with `cmake --build build --config Release --target _tests`. Visual Studio, Xcode and the Ninja Multi-Config generator that `build_linux.sh` uses are all multi-configuration, so `ctest` needs `-C` on every platform; without it, tests registered with plain `add_test()` lose their labels and report "Not Run". ## Where a test goes diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index c403152c4e..f79a3c54c2 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -85,4 +85,9 @@ add_subdirectory(fff_print) add_subdirectory(sla_print) add_subdirectory(filament_group) +# End-to-end checks of the orca-slicer binary. Linux only: they read result.json, which the CLI +# writes on Linux only. src/ is added before tests/, so the target is known here. +if (UNIX AND NOT APPLE AND TARGET OrcaSlicer) + add_subdirectory(cli) +endif () diff --git a/tests/cli/CMakeLists.txt b/tests/cli/CMakeLists.txt new file mode 100644 index 0000000000..6707159fb2 --- /dev/null +++ b/tests/cli/CMakeLists.txt @@ -0,0 +1,17 @@ +# Runs the real orca-slicer binary, so it needs the built app and resources/, not just build/tests. +# The CI unit-test job only receives build/tests, so the test carries the RequiresApp label that +# scripts/run_unit_tests.sh excludes. Run it with `ctest -C Release -L RequiresApp`. It also exits 77 +# (skipped) when the binary is missing. + +find_program(ORCA_CLI_TEST_PYTHON NAMES python3) +if (NOT ORCA_CLI_TEST_PYTHON) + message(STATUS "python3 not found, not registering the CLI tests") + return() +endif () + +add_test(NAME cli_strict_mode + COMMAND bash ${CMAKE_CURRENT_SOURCE_DIR}/test_cli_strict.sh $ ${ORCA_CLI_TEST_PYTHON}) +set_tests_properties(cli_strict_mode PROPERTIES + LABELS "CLI;RequiresApp" + SKIP_RETURN_CODE 77 + TIMEOUT 900) diff --git a/tests/cli/test_cli_strict.sh b/tests/cli/test_cli_strict.sh new file mode 100644 index 0000000000..6051511f23 --- /dev/null +++ b/tests/cli/test_cli_strict.sh @@ -0,0 +1,153 @@ +#!/usr/bin/env bash +# End-to-end check of the CLI --strict option against the real orca-slicer binary. +# +# A model with a large unsupported overhang, sliced with support off, raises the NON_CRITICAL +# "support needed" slicing warning. The CLI lists it in result.json's "warnings" array, and with +# --strict it also fails the run with CLI_SLICING_ERROR. --strict with --no-check is rejected up +# front, because --no-check skips that check. +# +# usage: test_cli_strict.sh +set -u + +BIN="${1:-}" +PY="${2:-python3}" +# 77 is the test's SKIP_RETURN_CODE. +[ -x "$BIN" ] || { echo "SKIP: orca-slicer binary not found: $BIN"; exit 77; } + +# From src/libslic3r/Utils.hpp. main() returns them, so the shell sees them modulo 256. +CLI_SUCCESS=0 +CLI_INVALID_PARAMS=-2 +CLI_SLICING_ERROR=-100 + +WORK="$(mktemp -d "${TMPDIR:-/tmp}/orca-cli-strict.XXXXXX")" +trap 'rm -rf "$WORK"' EXIT +mkdir -p "$WORK/datadir" + +# Standalone presets: without "inherits" the CLI loads them as-is, with no preset bundle. +cat > "$WORK/machine.json" <<'EOF' +{ + "type": "machine", + "from": "User", + "name": "CLI strict test printer", + "printable_area": ["0x0", "200x0", "200x200", "0x200"], + "printable_height": "100", + "layer_change_gcode": "G92 E0" +} +EOF +cat > "$WORK/process.json" <<'EOF' +{ + "type": "process", + "from": "User", + "name": "CLI strict test process", + "enable_support": "0", + "enforce_support_layers": "0" +} +EOF + +# A 40x40mm cap on an 8x8mm stem: the cap reaches ~22mm past the stem, beyond the 6mm +# cantilever limit of PrintObject::is_support_necessary(). +"$PY" - "$WORK/capital.stl" <<'EOF' +import sys + +def box(x0, y0, z0, x1, y1, z1): + v = [(x, y, z) for z in (z0, z1) for y in (y0, y1) for x in (x0, x1)] + # Faces wound counter-clockwise seen from outside: -z, +z, -y, +y, -x, +x. + for a, b, c, d in ((0, 2, 3, 1), (4, 5, 7, 6), (0, 1, 5, 4), (2, 6, 7, 3), (0, 4, 6, 2), (1, 3, 7, 5)): + yield v[a], v[b], v[c] + yield v[a], v[c], v[d] + +with open(sys.argv[1], "w") as f: + f.write("solid capital\n") + for tri in (*box(16, 16, 0, 24, 24, 13), *box(0, 0, 12, 40, 40, 14)): + f.write("facet normal 0 0 0\nouter loop\n") + for p in tri: + f.write("vertex %g %g %g\n" % p) + f.write("endloop\nendfacet\n") + f.write("endsolid capital\n") +EOF + +fails=0 +fail() { echo "FAIL: $*"; fails=$((fails + 1)); } + +# run [option...]: slice into $WORK/, keeping the log and the shell status there. +run() { + local out="$WORK/$1"; shift + mkdir -p "$out" + timeout 300 "$BIN" --datadir "$WORK/datadir" --load-settings "$WORK/machine.json;$WORK/process.json" \ + "$@" --slice 0 --outputdir "$out" "$WORK/capital.stl" > "$out/log" 2>&1 + echo $? > "$out/status" +} + +# expect_status +expect_status() { + local got; got="$(cat "$WORK/$1/status")" + [ "$got" -eq $(( $2 & 255 )) ] || fail "$1: shell status $got, want $(( $2 & 255 )) (code $2)" +} + +# expect_gcode yes|no +expect_gcode() { + if compgen -G "$WORK/$1/*.gcode" > /dev/null; then + [ "$2" = yes ] || fail "$1: G-code was exported" + else + [ "$2" = no ] || fail "$1: no G-code was exported" + fi +} + +# expect_result +expect_result() { + "$PY" - "$WORK/$1/result.json" "$2" "$3" "$4" <<'EOF' || fail "$1: result.json" +import json, sys + +path, want_rc, want_strict, want_warning = sys.argv[1], int(sys.argv[2]), sys.argv[3] == "true", sys.argv[4] +try: + with open(path) as f: + result = json.load(f) +except (OSError, ValueError) as e: + sys.exit("cannot read %s: %s" % (path, e)) + +errors = [] +if result.get("return_code") != want_rc: + errors.append("return_code %r, want %d" % (result.get("return_code"), want_rc)) +if result.get("strict_mode") is not want_strict: + errors.append("strict_mode %r, want %r" % (result.get("strict_mode"), want_strict)) +warnings = result.get("warnings") +if not isinstance(warnings, list): + errors.append("warnings %r is not a list" % (warnings,)) +else: + found = any(isinstance(w, dict) and w.get("class") == "slicing_warning_non_critical" for w in warnings) + if found != (want_warning == "some"): + errors.append("warnings %r, want %s slicing_warning_non_critical" % (warnings, want_warning)) +for e in errors: + print(e) +sys.exit(1 if errors else 0) +EOF +} + +echo "== without --strict the warning is listed and the slice succeeds" +run plain +expect_status plain $CLI_SUCCESS +expect_result plain $CLI_SUCCESS false some +expect_gcode plain yes + +echo "== --strict fails the run on the same warning, before G-code export" +run strict --strict +expect_status strict $CLI_SLICING_ERROR +expect_result strict $CLI_SLICING_ERROR true some +expect_gcode strict no + +echo "== --strict with --no-check is rejected before slicing" +run conflict --strict --no-check +expect_status conflict $CLI_INVALID_PARAMS +expect_result conflict $CLI_INVALID_PARAMS true none +expect_gcode conflict no +grep -q -- "--strict cannot be combined with --no-check" "$WORK/conflict/log" \ + || fail "conflict: error message missing" + +if [ "$fails" -ne 0 ]; then + for log in "$WORK"/*/log; do + echo "--- $log" + tail -n 40 "$log" + done + exit 1 +fi +echo "PASS" diff --git a/tests/fff_print/test_support_material.cpp b/tests/fff_print/test_support_material.cpp index 4564c0b034..97a9cea9e1 100644 --- a/tests/fff_print/test_support_material.cpp +++ b/tests/fff_print/test_support_material.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include @@ -128,6 +129,49 @@ TEST_CASE("Enforced support layers are generated", "[SupportMaterial]") REQUIRE(enforced.objects().front()->support_layers().size() > 0); } +// Support-needed statuses raised while slicing support_capital() with support off. The CLI lists these +// in result.json and fails on them under --strict. Collected under a lock: generate_support_material() +// runs on TBB workers. +static std::vector support_needed_statuses(bool no_check) +{ + Slic3r::Print print; + Slic3r::Model model; + Slic3r::Test::init_print({ support_capital() }, print, model, { + { "enable_support", 0 }, + { "enforce_support_layers", 0 } + }); + print.set_no_check_flag(no_check); + + std::mutex mutex; + std::vector statuses; + print.set_status_callback([&mutex, &statuses](const PrintBase::SlicingStatus &status) { + if (status.message_type != PrintStateBase::SlicingNeedSupportOn) + return; + std::lock_guard lock(mutex); + statuses.push_back(status); + }); + print.process(); + return statuses; +} + +TEST_CASE("An overhang sliced with support off reports that support is needed", "[SupportMaterial]") +{ + // The 40mm cap reaches ~22mm past its 8mm stem, beyond the 6mm cantilever limit of + // PrintObject::is_support_necessary(). + const std::vector statuses = support_needed_statuses(false); + REQUIRE(! statuses.empty()); + for (const PrintBase::SlicingStatus &status : statuses) { + // The CLI only considers step warnings (warning_step != -1), and --strict only NON_CRITICAL ones. + CHECK(status.warning_level == PrintStateBase::WarningLevel::NON_CRITICAL); + CHECK(status.warning_step != -1); + } +} + +TEST_CASE("The no-check flag skips the support-needed check", "[SupportMaterial]") +{ + CHECK(support_needed_statuses(true).empty()); +} + SCENARIO("Support layer Z honors contact distance", "[SupportMaterial]") { // Box h = 20mm, hole bottom at 5mm, hole height 10mm (top edge at 15mm).