Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
/*.dat
*.sdf
.tmp/
.tmp\
Copy link
Copy Markdown
Collaborator

@Mosch0512 Mosch0512 May 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated change.

This .tmp\ deletion isn't related to the editor-leak test. Per docs/CODING_RULES.md §10 (keep diffs focused on one concern), please split this into its own commit or its own PR. The change itself is fine - .tmp\ was never valid gitignore syntax, the .tmp/ line above already handles the directory - but it doesn't belong here.

*.bak
*.sbr
*.tlog
Expand Down
2 changes: 2 additions & 0 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,5 @@ function(mu_add_test)
endfunction()

add_subdirectory(text)

add_subdirectory(editor)
66 changes: 66 additions & 0 deletions tests/editor/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
# Test tree. New test modules go in their own subdirectory and call
# add_subdirectory() below. Keep the entry point here minimal — module
# CMakeLists.txt files own their own sources, link, and call mu_add_test().

include(third_party/doctest/doctest.cmake)

Comment thread
Mosch0512 marked this conversation as resolved.
Outdated
# When cross-compiling Windows binaries on a non-Windows host (CI's Linux
# runner, WSL/MinGW), wine has to launch the .exe. Setting this once makes
# both add_test and doctest_discover_tests pick it up via the
# CROSSCOMPILING_EMULATOR target property.
if(WIN32 AND NOT CMAKE_HOST_WIN32 AND NOT CMAKE_CROSSCOMPILING_EMULATOR)
find_program(WINE_EXECUTABLE wine)
if(NOT WINE_EXECUTABLE)
message(FATAL_ERROR
"BUILD_TESTING=ON on a non-Windows host requires wine to run "
"the cross-compiled test binaries. Install it (Ubuntu/Debian: "
"sudo apt-get install wine wine32) or configure with "
"-DBUILD_TESTING=OFF.")
endif()
set(CMAKE_CROSSCOMPILING_EMULATOR ${WINE_EXECUTABLE} CACHE STRING "" FORCE)
endif()

add_library(mu_test_main STATIC main.cpp)
target_include_directories(mu_test_main PUBLIC third_party/doctest)
target_compile_features(mu_test_main PUBLIC cxx_std_20)
# Test sources contain non-ASCII wide string literals. MSVC defaults to the
# system codepage when reading sources, which mangles UTF-8 characters into
# wrong codepoints and breaks every non-Latin language test. /utf-8 forces
# both the source charset and the narrow execution charset to UTF-8; the wide
# execution charset stays UTF-16 (Windows wchar_t).
if(MSVC)
target_compile_options(mu_test_main PUBLIC /utf-8)
endif()
Comment thread
Mosch0512 marked this conversation as resolved.
Outdated

# Helper for module CMakeLists. Usage:
# mu_add_test(NAME <test_name> SOURCES a.cpp b.cpp LINK_LIBS lib1 lib2)
# Registers each TEST_CASE inside the binary as its own CTest entry, so
# failures point at the specific case in CI logs.
function(mu_add_test)
cmake_parse_arguments(MAT "" "NAME" "SOURCES;LINK_LIBS" ${ARGN})
add_executable(${MAT_NAME} ${MAT_SOURCES})
target_link_libraries(${MAT_NAME} PRIVATE mu_test_main ${MAT_LINK_LIBS})
target_include_directories(${MAT_NAME} PRIVATE
${CMAKE_SOURCE_DIR}/src/source
${CMAKE_CURRENT_SOURCE_DIR}/third_party/doctest
)
if(MSVC)
target_compile_options(${MAT_NAME} PRIVATE /utf-8)
endif()
doctest_discover_tests(${MAT_NAME})
endfunction()

add_subdirectory(text)

Comment thread
Mosch0512 marked this conversation as resolved.
Outdated
# Add Editor Leak Test (only runs when ENABLE_EDITOR is OFF)
if(NOT ENABLE_EDITOR)
# Generate a text file containing all sources of the Main target
file(GENERATE
OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/main_sources.txt"
CONTENT "$<JOIN:$<TARGET_PROPERTY:Main,SOURCES>,\n>")

# Add a test that runs a Python script to verify MuEditor isn't in those sources
find_package(Python3 COMPONENTS Interpreter REQUIRED)
add_test(NAME test_editor_leak
Comment thread
Mosch0512 marked this conversation as resolved.
Outdated
COMMAND Python3::Interpreter ${CMAKE_CURRENT_SOURCE_DIR}/editor/test_leak.py "${CMAKE_CURRENT_BINARY_DIR}/main_sources.txt")
Comment thread
Mosch0512 marked this conversation as resolved.
Outdated
endif()
28 changes: 28 additions & 0 deletions tests/editor/test_leak.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import sys
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Import the os module to enable robust path handling. This is necessary for calculating relative paths to the project root, which helps avoid false positives in the leak check.

Suggested change
import sys
import sys
import os


def main():
if len(sys.argv) < 2:
print("Usage: test_leak.py <sources_file>")
sys.exit(1)

sources_file = sys.argv[1]

with open(sources_file, 'r', encoding='utf-8') as f:
sources = f.read().splitlines()

# Normalise to forward slashes so the path-segment check is
# platform-independent and won't fire just because the repository
# happens to be cloned inside a directory called "MuEditor".
leaked_files = [s for s in sources if '/MuEditor/' in s.replace('\\', '/')]
Comment on lines +13 to +16
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current path matching logic is susceptible to false positives if the repository is cloned into a directory named MuEditor (e.g., /home/user/MuEditor/src/source/main.cpp), which contradicts the intent stated in the preceding comment. Since the MuEditor directory is located inside src, using a more specific pattern like /src/MuEditor/ would be more robust. Additionally, the leading slash in '/MuEditor/' would cause the test to miss relative paths (e.g., MuEditor/file.cpp), though the current GLOB_RECURSE in CMake produces absolute paths.

Suggested change
leaked_files = [s for s in sources if '/MuEditor/' in s.replace('\\', '/')]
leaked_files = [s for s in sources if '/src/MuEditor/' in s.replace('\\', '/')]

Comment on lines +13 to +16
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current check for '/MuEditor/' will trigger a false positive if the repository is cloned into a directory that contains MuEditor in its path (e.g., /home/user/MuEditor/project/...). Although the comment suggests this is handled, the implementation actually checks the entire absolute path.

To fix this, calculate the path of each source file relative to the project root and check if MuEditor is a directory segment within that relative path.

Suggested change
# Normalise to forward slashes so the path-segment check is
# platform-independent and won't fire just because the repository
# happens to be cloned inside a directory called "MuEditor".
leaked_files = [s for s in sources if '/MuEditor/' in s.replace('\\', '/')]
# Use relative paths to the project root to avoid false positives if the
# repository itself is cloned into a directory named "MuEditor".
project_root = os.path.abspath(os.path.join(os.path.dirname(__file__), '..', '..'))
leaked_files = [s for s in sources if 'MuEditor' in os.path.relpath(s, project_root).replace('\\', '/').split('/')]

Comment on lines +13 to +16
Copy link
Copy Markdown
Collaborator

@Mosch0512 Mosch0512 May 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test name overstates coverage.

This catches file-path leaks (sources whose path contains /MuEditor/). It does NOT catch the other (and arguably more common) way editor code reaches the binary: #ifdef _EDITOR blocks inside src/source/ files. A quick grep finds 59 files under src/source/ that gate code on _EDITOR. If someone accidentally removes the if(ENABLE_EDITOR) guard around the _EDITOR compile definition (src/CMakeLists.txt:217), every one of those #ifdef blocks compiles in and this test still reports PASS.

Two options:

  • Narrow the name to match what it does: rename the CTest entry to something like test_editor_dir_not_in_main_sources and add a one-line docstring saying it only covers path-level leaks.
  • Broaden the test to match its name: add a second assertion that _EDITOR is not in $<TARGET_PROPERTY:Main,COMPILE_DEFINITIONS> when ENABLE_EDITOR=OFF.

Either is fine - but the current name + scope mismatch will mislead future readers.


if leaked_files:
print("FAIL: MuEditor leaked into the build! The following editor files were found in the Main target sources:")
for f in leaked_files:
print(f" - {f}")
Comment on lines +10 to +21
sys.exit(1)
else:
print("PASS: No MuEditor files leaked into the build.")
sys.exit(0)

if __name__ == '__main__':
main()
Loading