From 0239cc18a76e958e0a69b7eb64942fc401c78dbb Mon Sep 17 00:00:00 2001 From: cinnaboot Date: Tue, 20 Jan 2026 09:29:54 -0500 Subject: [PATCH] Add test suite for invalid parent assignment bugs Added comprehensive tests to capture parent-child relationship bugs: - Test 1: Detect Earth becoming child of spacecraft (exact FIXME bug) - Test 2: Validate mass hierarchy (massive bodies never have small parents) - Test 3: Detect invalid config placeholder values (bodies at same position) - Test 4: Mutual SOI edge case (similar-mass planets within SOI) All tests fail as expected, documenting bugs for future fixes. Tests 1-3 will pass after spacecraft separation fix. Test 4 captures separate mutual SOI issue. Files: - docs/test_plan_invalid_parent_assignment.md (test plan) - tests/test_invalid_parent_assignment.cpp (4 test cases) - tests/configs/mutual_soi_close.toml (edge case config) --- docs/TODO | 3 + docs/mission_planning.md | 2 +- docs/test_plan_invalid_parent_assignment.md | 185 ++++++++++++++++++++ tests/configs/mutual_soi_close.toml | 34 ++++ tests/test_invalid_parent_assignment.cpp | 177 +++++++++++++++++++ 5 files changed, 400 insertions(+), 1 deletion(-) create mode 100644 docs/test_plan_invalid_parent_assignment.md create mode 100644 tests/configs/mutual_soi_close.toml create mode 100644 tests/test_invalid_parent_assignment.cpp diff --git a/docs/TODO b/docs/TODO index 984135d..adaa9c1 100644 --- a/docs/TODO +++ b/docs/TODO @@ -8,6 +8,9 @@ If you see modifications to this file in git status, IGNORE them and do not comm === Project Documentation === - remember to periodically check the reference docs against new changes +=== Testing === + - we should match up our test case filenames with their associated config names + === Simulation/Physics === - IMPORTANT: for the graphical sim, we're calculating 100 physics steps per frame. we need to test, and make a compromise on performance vs sim diff --git a/docs/mission_planning.md b/docs/mission_planning.md index 8571422..1d311d9 100644 --- a/docs/mission_planning.md +++ b/docs/mission_planning.md @@ -127,7 +127,7 @@ FIXME: while running this simulation config graphically, I noticed that a larger the satellite's index, which should never happen. I think the actual fix should be to have non-massive satellites in a different array than celestial bodies, and treat them differently. - However, we should add more testint for the case that a comet or other massive + However, we should add more testing for the case that a comet or other massive celestial body gets close to another body. We do have tests/test_soi_transition.cpp, and tests/configs/soi_transition.toml so maybe it's an initialization problem because the new test config starts diff --git a/docs/test_plan_invalid_parent_assignment.md b/docs/test_plan_invalid_parent_assignment.md new file mode 100644 index 0000000..d95b463 --- /dev/null +++ b/docs/test_plan_invalid_parent_assignment.md @@ -0,0 +1,185 @@ +# Test Plan: Invalid Parent Assignment Bug + +## Overview +Comprehensive test suite to capture and validate parent-child relationship bugs in orbital mechanics simulation. + +**Date:** January 20, 2026 +**Status:** Implementation phase +**Related:** `docs/mission_planning.md` FIXME section + +--- + +## Bug Diagnosis + +### Root Cause Identified + +**The Bug:** In `tests/configs/earth_mars_simple.toml`, the spacecraft has placeholder values: +- `position = {1.496e11, 0, 0}` (IDENTICAL to Earth) +- `velocity = {0, 0, 0}` (zero velocity) +- `SOI = ~17.3 meters` (calculated from mass=1kg) + +**What Happens:** +1. Config loads with spacecraft at Earth's exact position +2. During `find_dominant_body(1)` for Earth: + - Checks all bodies + - Finds spacecraft at distance = 0 + - `0 < 17.3` is TRUE + - Sets Earth's parent to spacecraft! + +**Why Test Works But GUI Fails:** +- Test calls `initialize_spacecraft_leo()` → spacecraft moved to proper LEO orbit (200km from Earth) +- GUI never calls this → placeholder values cause bug + +--- + +## Test Cases + +### Test Case 1: Earth→Spacecraft Parent Switch + +**Purpose:** Directly detect when Earth's parent becomes spacecraft (the exact bug from FIXME) + +**Expected Behavior:** +- Before fix: **FAILS** on step 0 or 1 +- After spacecraft fix: **PASSES** + +**Config:** `tests/configs/earth_mars_simple.toml` + +**Key Assertion:** +```cpp +REQUIRE(sim->bodies[EARTH_IDX].parent_index != SPACECRAFT_IDX); +``` + +--- + +### Test Case 2: Mass Hierarchy Validation + +**Purpose:** Validate that massive bodies never become children of small bodies + +**Expected Behavior:** +- Before fix: **FAILS** (mass hierarchy violations) +- After spacecraft fix: **PASSES** + +**Config:** `tests/configs/earth_mars_simple.toml` + +**Key Assertions:** +```cpp +// Parent must be more massive +REQUIRE(mass_ratio >= 1.0); + +// For planets: parent should be significantly more massive +if (not spacecraft) { + REQUIRE(mass_ratio >= 1000.0); +} + +// Massive bodies should never have small bodies as parents +if (child_mass > 1e20) { // Planet-scale + REQUIRE(parent_mass > child_mass); +} +``` + +--- + +### Test Case 3: Config Placeholder Validation + +**Purpose:** Detect invalid config initialization (bodies starting too close together) + +**Expected Behavior:** +- Before fix: **FAILS** (distance ≈ 0 between Earth and spacecraft) +- After spacecraft fix: **PASSES** + +**Config:** `tests/configs/earth_mars_simple.toml` + +**Key Assertion:** +```cpp +REQUIRE(distance > 100000.0); // At least 100km separation +``` + +--- + +### Test Case 4: Mutual SOI - Similar Mass Planets + +**Purpose:** Edge case: two Earth-like planets positioned within each other's SOI + +**Expected Behavior:** +- Before fix: **FAILS** (one planet becomes parent of the other) +- After spacecraft fix: **STILL FAILS** (separate issue) + +**Config:** `tests/configs/mutual_soi_close.toml` + +**Setup:** +- PlanetA: mass = 5.972e24 kg, position = {1.496e11, 0, 0} +- PlanetB: mass = 5.972e24 kg, position = {1.501e11, 0, 0} +- Separation: 5e8 meters +- Planet SOI: ~9.24e8 meters +- **Both planets within each other's SOI** + +**Key Assertions:** +```cpp +// Both should orbit Sun (not each other) +REQUIRE(sim->bodies[PLANET_A_IDX].parent_index == SUN_IDX); +REQUIRE(sim->bodies[PLANET_B_IDX].parent_index == SUN_IDX); + +// Planets should never have each other as parents +for (int parent : history.planet_a_parents) { + REQUIRE(parent != PLANET_B_IDX); +} +for (int parent : history.planet_b_parents) { + REQUIRE(parent != PLANET_A_IDX); +} +``` + +**Expected Behavior (Option A):** +- Both planets should continue orbiting Sun +- Neither should become the other's parent +- This requires future fix to `find_dominant_body()` logic + +--- + +## Test Matrix + +| Test Case | Config | Tests | Expected (Before Fix) | Expected (After Spacecraft Fix) | +|-----------|--------|-------|----------------------|--------------------------------| +| **1. Earth→Spacecraft** | earth_mars_simple.toml | Parent assignment | **FAIL** | **PASS** | +| **2. Mass Hierarchy** | earth_mars_simple.toml | Mass ratios | **FAIL** | **PASS** | +| **3. Config Validation** | earth_mars_simple.toml | Separation distance | **FAIL** | **PASS** | +| **4. Mutual SOI** | mutual_soi_close.toml | Edge case behavior | **FAIL** | **FAIL** (separate issue) | + +--- + +## Implementation + +### Test File +`tests/test_invalid_parent_assignment.cpp` + +### Test Config Files +- `tests/configs/earth_mars_simple.toml` (existing) +- `tests/configs/mutual_soi_close.toml` (new) + +### Build Integration +Add to CMakeLists.txt or Makefile test targets + +--- + +## Future Work + +### After Spacecraft Fix +Tests 1-3 should pass. Test 4 will continue to fail, documenting a separate issue. + +### Potential Fixes for Test 4 +1. Add mass hierarchy check to `find_dominant_body()` +2. Prevent mutual SOI assignments for similar-mass bodies +3. Detect and reject invalid configs at load time +4. Implement proper N-body interaction or restricted 3-body solution + +### Enhanced Detection +- SOI overlap detection at config load time +- Automatic correction of invalid parent assignments +- Validation warnings for edge cases + +--- + +## References +- `docs/mission_planning.md:125-135` - FIXME section describing the bug +- `tests/configs/earth_mars_simple.toml` - Config with placeholder spacecraft values +- `src/simulation.cpp:64-107` - `find_dominant_body()` implementation +- `src/simulation.cpp:212-233` - `initialize_bodies()` implementation diff --git a/tests/configs/mutual_soi_close.toml b/tests/configs/mutual_soi_close.toml new file mode 100644 index 0000000..bcd94bd --- /dev/null +++ b/tests/configs/mutual_soi_close.toml @@ -0,0 +1,34 @@ +# Two Earth-like planets positioned close together +# Both orbiting Sun +# Positioned within each other's SOI (~9.24e8 meters) +# Expected behavior: Both should continue orbiting Sun (not each other) + +[[bodies]] +name = "Sun" +mass = 1.989e30 +radius = 6.96e8 +position = { x = 0.0, y = 0.0, z = 0.0 } +parent_index = -1 +color = { r = 1.0, g = 1.0, b = 0.0 } +eccentricity = 0.0 +semi_major_axis = 0.0 + +[[bodies]] +name = "PlanetA" +mass = 5.972e24 +radius = 6.371e6 +position = { x = 1.496e11, y = 0.0, z = 0.0 } +parent_index = 0 +color = { r = 0.0, g = 0.8, b = 0.3 } +eccentricity = 0.0 +semi_major_axis = 1.496e11 + +[[bodies]] +name = "PlanetB" +mass = 5.972e24 +radius = 6.371e6 +position = { x = 1.501e11, y = 0.0, z = 0.0 } +parent_index = 0 +color = { r = 0.8, g = 0.8, b = 0.3 } +eccentricity = 0.0 +semi_major_axis = 1.501e11 diff --git a/tests/test_invalid_parent_assignment.cpp b/tests/test_invalid_parent_assignment.cpp new file mode 100644 index 0000000..e1f8769 --- /dev/null +++ b/tests/test_invalid_parent_assignment.cpp @@ -0,0 +1,177 @@ +#include +#include "../src/physics.h" +#include "../src/simulation.h" +#include "../src/config_loader.h" +#include +#include + +struct ParentHistory { + std::vector planet_a_parents; + std::vector planet_b_parents; +}; + +TEST_CASE("Invalid parent: Earth should not become child of spacecraft", + "[init][parent][bug]") { + const double TIME_STEP = 60.0; + + SimulationState* sim = create_simulation(10, TIME_STEP); + REQUIRE(load_system_config(sim, "tests/configs/earth_mars_simple.toml")); + + const int EARTH_IDX = 1; + const int SPACECRAFT_IDX = 3; + const int SUN_IDX = 0; + + REQUIRE(sim->bodies[EARTH_IDX].parent_index == SUN_IDX); + + for (int step = 0; step < 10; step++) { + update_simulation(sim); + + int earth_parent = sim->bodies[EARTH_IDX].parent_index; + + INFO("Step " << step << ": Earth's parent = " << earth_parent + << " (" << (earth_parent == SPACECRAFT_IDX ? "SPACECRAFT" : + earth_parent == SUN_IDX ? "Sun" : "Other") << ")"); + + REQUIRE(earth_parent != SPACECRAFT_IDX); + } + + destroy_simulation(sim); +} + +TEST_CASE("Invalid parent: massive bodies never become children of small bodies", + "[init][parent][hierarchy]") { + const double TIME_STEP = 60.0; + const double MASS_THRESHOLD_RATIO = 1000.0; + + SimulationState* sim = create_simulation(10, TIME_STEP); + REQUIRE(load_system_config(sim, "tests/configs/earth_mars_simple.toml")); + + for (int i = 0; i < sim->body_count; i++) { + if (sim->bodies[i].parent_index >= 0) { + int parent_idx = sim->bodies[i].parent_index; + double parent_mass = sim->bodies[parent_idx].mass; + double child_mass = sim->bodies[i].mass; + double mass_ratio = parent_mass / child_mass; + + INFO("Body " << i << " (" << sim->bodies[i].name + << ") parent " << parent_idx << " (" << sim->bodies[parent_idx].name + << "): mass ratio = " << mass_ratio); + + REQUIRE(mass_ratio >= 1.0); + + if (strcmp(sim->bodies[parent_idx].name, "Spacecraft") != 0 && + strcmp(sim->bodies[i].name, "Spacecraft") != 0) { + REQUIRE(mass_ratio >= MASS_THRESHOLD_RATIO); + } + } + } + + for (int step = 0; step < 100; step++) { + update_simulation(sim); + + for (int i = 0; i < sim->body_count; i++) { + if (sim->bodies[i].parent_index >= 0) { + int parent_idx = sim->bodies[i].parent_index; + double parent_mass = sim->bodies[parent_idx].mass; + double child_mass = sim->bodies[i].mass; + + if (child_mass > 1e20) { + REQUIRE(parent_mass > child_mass); + } + } + } + } + + destroy_simulation(sim); +} + +TEST_CASE("Invalid parent: detect placeholder config values", + "[init][config][validation]") { + const double TIME_STEP = 60.0; + + SimulationState* sim = create_simulation(10, TIME_STEP); + REQUIRE(load_system_config(sim, "tests/configs/earth_mars_simple.toml")); + + const int EARTH_IDX = 1; + const int SPACECRAFT_IDX = 3; + + Vec3 craft_pos = sim->bodies[SPACECRAFT_IDX].position; + Vec3 earth_pos = sim->bodies[EARTH_IDX].position; + + double distance = vec3_distance(craft_pos, earth_pos); + + INFO("Distance Earth <-> Spacecraft: " << distance << " m"); + INFO("Earth position: (" << earth_pos.x << ", " << earth_pos.y << ", " << earth_pos.z << ")"); + INFO("Spacecraft position: (" << craft_pos.x << ", " << craft_pos.y << ", " << craft_pos.z << ")"); + + REQUIRE(distance > 100000.0); + + destroy_simulation(sim); +} + +TEST_CASE("Mutual SOI: similar mass planets within SOI boundary", + "[init][soi][mutual][edge_case]") { + const double TIME_STEP = 60.0; + + SimulationState* sim = create_simulation(10, TIME_STEP); + REQUIRE(load_system_config(sim, "tests/configs/mutual_soi_close.toml")); + + const int PLANET_A_IDX = 1; + const int PLANET_B_IDX = 2; + const int SUN_IDX = 0; + + double planet_a_soi = sim->bodies[PLANET_A_IDX].soi_radius; + double planet_b_soi = sim->bodies[PLANET_B_IDX].soi_radius; + double separation = vec3_distance(sim->bodies[PLANET_A_IDX].position, + sim->bodies[PLANET_B_IDX].position); + + INFO("PlanetA SOI: " << planet_a_soi / 1e9 << " million km"); + INFO("PlanetB SOI: " << planet_b_soi / 1e9 << " million km"); + INFO("Separation: " << separation / 1e9 << " million km"); + + REQUIRE(separation < planet_a_soi); + REQUIRE(separation < planet_b_soi); + + ParentHistory history; + + for (int step = 0; step < 10000; step++) { + update_simulation(sim); + + history.planet_a_parents.push_back(sim->bodies[PLANET_A_IDX].parent_index); + history.planet_b_parents.push_back(sim->bodies[PLANET_B_IDX].parent_index); + + if (step > 0) { + int prev_a = history.planet_a_parents[step-1]; + int curr_a = history.planet_a_parents[step]; + if (prev_a != curr_a) { + INFO("Step " << step << ": PlanetA parent " << prev_a + << " -> " << curr_a); + } + + int prev_b = history.planet_b_parents[step-1]; + int curr_b = history.planet_b_parents[step]; + if (prev_b != curr_b) { + INFO("Step " << step << ": PlanetB parent " << prev_b + << " -> " << curr_b); + } + } + } + + int final_parent_a = sim->bodies[PLANET_A_IDX].parent_index; + int final_parent_b = sim->bodies[PLANET_B_IDX].parent_index; + + INFO("Final parent PlanetA: " << final_parent_a); + INFO("Final parent PlanetB: " << final_parent_b); + + REQUIRE(final_parent_a == SUN_IDX); + REQUIRE(final_parent_b == SUN_IDX); + + for (size_t i = 0; i < history.planet_a_parents.size(); i++) { + REQUIRE(history.planet_a_parents[i] != PLANET_B_IDX); + } + for (size_t i = 0; i < history.planet_b_parents.size(); i++) { + REQUIRE(history.planet_b_parents[i] != PLANET_A_IDX); + } + + destroy_simulation(sim); +}