Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
265 changes: 265 additions & 0 deletions src/MCPServer_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <QTcpSocket>
#include <QSignalSpy>
#include <QElapsedTimer>
#include <QDir>
#include <memory>
#include "MCPServer.h"
#include "Manager.h"
Expand Down Expand Up @@ -2224,3 +2225,267 @@ TEST_F(MCPServerTest, AnimateWithOnlyRoll)
EXPECT_TRUE(text.contains("Started animation"));
EXPECT_TRUE(text.contains("roll: 90"));
}

// ==========================================================================
// NEW: In-memory entity success path tests
// ==========================================================================

TEST_F(MCPServerTest, GetMeshInfoWithInMemoryEntity)
{
if (!canLoadMeshFiles()) { GTEST_SKIP() << "Skipping: entity creation not supported without render window"; }

auto mesh = createInMemoryTriangleMesh("MCPMeshInfoTriangle");
auto* sceneMgr = Manager::getSingleton()->getSceneMgr();
auto* node = Manager::getSingleton()->addSceneNode("MCPMeshInfoNode");
auto* entity = sceneMgr->createEntity("MCPMeshInfoEntity", mesh);
node->attachObject(entity);

QJsonObject result = server->callTool("get_mesh_info", QJsonObject());
EXPECT_FALSE(isError(result));
QString text = getResultText(result);
EXPECT_TRUE(text.contains("Vertices"));
}

TEST_F(MCPServerTest, TransformMeshPositionWithInMemoryEntity)
{
if (!canLoadMeshFiles()) { GTEST_SKIP() << "Skipping: entity creation not supported without render window"; }

auto mesh = createInMemoryTriangleMesh("MCPTransformTriangle");
auto* sceneMgr = Manager::getSingleton()->getSceneMgr();
auto* node = Manager::getSingleton()->addSceneNode("MCPTransformNode");
auto* entity = sceneMgr->createEntity("MCPTransformEntity", mesh);
node->attachObject(entity);

QJsonObject transArgs;
transArgs["name"] = "MCPTransformNode";
transArgs["position"] = QJsonArray{5.0, 10.0, 15.0};
QJsonObject result = server->callTool("transform_mesh", transArgs);
EXPECT_FALSE(isError(result));
EXPECT_TRUE(getResultText(result).contains("position"));

// Verify position was actually set
EXPECT_NEAR(node->getPosition().x, 5.0f, 0.01f);
EXPECT_NEAR(node->getPosition().y, 10.0f, 0.01f);
EXPECT_NEAR(node->getPosition().z, 15.0f, 0.01f);
}

TEST_F(MCPServerTest, TransformMeshRotationWithInMemoryEntity)
{
if (!canLoadMeshFiles()) { GTEST_SKIP() << "Skipping: entity creation not supported without render window"; }

auto mesh = createInMemoryTriangleMesh("MCPRotateTriangle");
auto* sceneMgr = Manager::getSingleton()->getSceneMgr();
auto* node = Manager::getSingleton()->addSceneNode("MCPRotateNode");
auto* entity = sceneMgr->createEntity("MCPRotateEntity", mesh);
node->attachObject(entity);

QJsonObject transArgs;
transArgs["name"] = "MCPRotateNode";
transArgs["rotation"] = QJsonArray{45.0, 90.0, 0.0};
QJsonObject result = server->callTool("transform_mesh", transArgs);
EXPECT_FALSE(isError(result));
EXPECT_TRUE(getResultText(result).contains("rotation"));
}

TEST_F(MCPServerTest, TransformMeshScaleWithInMemoryEntity)
{
if (!canLoadMeshFiles()) { GTEST_SKIP() << "Skipping: entity creation not supported without render window"; }

auto mesh = createInMemoryTriangleMesh("MCPScaleTriangle");
auto* sceneMgr = Manager::getSingleton()->getSceneMgr();
auto* node = Manager::getSingleton()->addSceneNode("MCPScaleNode");
auto* entity = sceneMgr->createEntity("MCPScaleEntity", mesh);
node->attachObject(entity);

QJsonObject transArgs;
transArgs["name"] = "MCPScaleNode";
transArgs["scale"] = QJsonArray{2.0, 3.0, 4.0};
QJsonObject result = server->callTool("transform_mesh", transArgs);
EXPECT_FALSE(isError(result));
EXPECT_TRUE(getResultText(result).contains("scale"));

EXPECT_NEAR(node->getScale().x, 2.0f, 0.01f);
EXPECT_NEAR(node->getScale().y, 3.0f, 0.01f);
EXPECT_NEAR(node->getScale().z, 4.0f, 0.01f);
}

TEST_F(MCPServerTest, ExportMeshWithInMemoryEntity)
{
if (!canLoadMeshFiles()) { GTEST_SKIP() << "Skipping: entity creation not supported without render window"; }

auto mesh = createInMemoryTriangleMesh("MCPExportTriangle");
auto* sceneMgr = Manager::getSingleton()->getSceneMgr();
auto* node = Manager::getSingleton()->addSceneNode("MCPExportNode");
auto* entity = sceneMgr->createEntity("MCPExportEntity", mesh);
node->attachObject(entity);

SelectionSet::getSingleton()->selectOne(node);

QJsonObject args;
const QString exportBase = QDir(QDir::tempPath()).filePath("mcp_inmemory_export");
args["path"] = exportBase + ".obj";
QJsonObject result = server->callTool("export_mesh", args);
EXPECT_FALSE(isError(result));

QFile::remove(exportBase + ".obj");
QFile::remove(exportBase + ".material");
QFile::remove(exportBase + ".mtl");
Comment on lines +2325 to +2332

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, check the exact code at lines 2325-2332 and surrounding context
head -2332 src/MCPServer_test.cpp | tail -20

Repository: fernandotonon/QtMeshEditor

Length of output: 912


🏁 Script executed:

# Check the include section to see what's already included
head -20 src/MCPServer_test.cpp

Repository: fernandotonon/QtMeshEditor

Length of output: 572


🏁 Script executed:

# Search for other temp file patterns and QTemporaryDir usage in the file
grep -n "QTemporaryDir\|QDir::tempPath\|tempPath\|mcp_inmemory" src/MCPServer_test.cpp | head -30

Repository: fernandotonon/QtMeshEditor

Length of output: 161


🏁 Script executed:

# Check if this test function is isolated or run in parallel
grep -n "^TEST\|^class.*Test" src/MCPServer_test.cpp | grep -B2 -A2 "2325\|export_mesh"

Repository: fernandotonon/QtMeshEditor

Length of output: 52


🏁 Script executed:

# Verify what files are cleaned up after the test
sed -n '2320,2340p' src/MCPServer_test.cpp

Repository: fernandotonon/QtMeshEditor

Length of output: 765


Use unique temporary directories to avoid test file collisions.

Line 2325 uses a fixed basename in the shared system temp directory. Concurrent or repeated test runs can cause file conflicts. Use QTemporaryDir to create a unique per-test directory instead.

Proposed fix
+#include <QTemporaryDir>
 ...
-    const QString exportBase = QDir(QDir::tempPath()).filePath("mcp_inmemory_export");
+    QTemporaryDir tempDir;
+    ASSERT_TRUE(tempDir.isValid());
+    const QString exportBase = QDir(tempDir.path()).filePath("mcp_inmemory_export");
     args["path"] = exportBase + ".obj";
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const QString exportBase = QDir(QDir::tempPath()).filePath("mcp_inmemory_export");
args["path"] = exportBase + ".obj";
QJsonObject result = server->callTool("export_mesh", args);
EXPECT_FALSE(isError(result));
QFile::remove(exportBase + ".obj");
QFile::remove(exportBase + ".material");
QFile::remove(exportBase + ".mtl");
QTemporaryDir tempDir;
ASSERT_TRUE(tempDir.isValid());
const QString exportBase = QDir(tempDir.path()).filePath("mcp_inmemory_export");
args["path"] = exportBase + ".obj";
QJsonObject result = server->callTool("export_mesh", args);
EXPECT_FALSE(isError(result));
QFile::remove(exportBase + ".obj");
QFile::remove(exportBase + ".material");
QFile::remove(exportBase + ".mtl");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/MCPServer_test.cpp` around lines 2325 - 2332, The test currently
constructs a fixed exportBase in the shared temp dir (exportBase =
QDir(QDir::tempPath()).filePath("mcp_inmemory_export")) and writes/cleans files
which can collide; replace this with a QTemporaryDir instance and use its path()
to build a unique per-test directory for args["path"] (used by
server->callTool("export_mesh", args)) and adjust cleanup to rely on the
QTemporaryDir lifetime (or explicitly remove within that temp dir) so the export
.obj/.material/.mtl filenames are created inside the temporary directory rather
than a shared basename.

SelectionSet::getSingleton()->clear();
}

TEST_F(MCPServerTest, SetMaterialOnInMemoryEntity)
{
if (!canLoadMeshFiles()) { GTEST_SKIP() << "Skipping: entity creation not supported without render window"; }

// Create material
QJsonObject matArgs;
matArgs["name"] = "InMemMatTest";
server->callTool("create_material", matArgs);

// Create entity
auto mesh = createInMemoryTriangleMesh("MCPMatTriangle");
auto* sceneMgr = Manager::getSingleton()->getSceneMgr();
auto* node = Manager::getSingleton()->addSceneNode("MCPMatNode");
auto* entity = sceneMgr->createEntity("MCPMatEntity", mesh);
node->attachObject(entity);

// Apply material
QJsonObject applyArgs;
applyArgs["material"] = "InMemMatTest";
applyArgs["mesh"] = "MCPMatEntity";
QJsonObject result = server->callTool("apply_material", applyArgs);
EXPECT_FALSE(isError(result));
EXPECT_TRUE(getResultText(result).contains("Applied material"));
}

TEST_F(MCPServerTest, GetSceneInfoWithInMemoryEntities)
{
if (!canLoadMeshFiles()) { GTEST_SKIP() << "Skipping: entity creation not supported without render window"; }

auto mesh1 = createInMemoryTriangleMesh("MCPSceneInfo1");
auto mesh2 = createInMemoryTriangleMesh("MCPSceneInfo2");
auto* sceneMgr = Manager::getSingleton()->getSceneMgr();

auto* node1 = Manager::getSingleton()->addSceneNode("MCPSceneNode1");
auto* entity1 = sceneMgr->createEntity("MCPSceneEntity1", mesh1);
node1->attachObject(entity1);

auto* node2 = Manager::getSingleton()->addSceneNode("MCPSceneNode2");
auto* entity2 = sceneMgr->createEntity("MCPSceneEntity2", mesh2);
node2->attachObject(entity2);

QJsonObject result = server->callTool("get_scene_info", QJsonObject());
EXPECT_FALSE(isError(result));
QString text = getResultText(result);
EXPECT_TRUE(text.contains("Scene Information"));
EXPECT_TRUE(text.contains("MCPSceneNode1"));
EXPECT_TRUE(text.contains("MCPSceneNode2"));
}

TEST_F(MCPServerTest, ListSkeletalAnimationsWithSkeletonEntity)
{
if (!canLoadMeshFiles()) { GTEST_SKIP() << "Skipping: entity creation not supported without render window"; }

Ogre::Entity* entity = createAnimatedTestEntity("MCPSkelAnimEntity");
ASSERT_NE(entity, nullptr);

QJsonObject result = server->callTool("list_skeletal_animations", QJsonObject());
EXPECT_FALSE(isError(result));
QString text = getResultText(result);
EXPECT_TRUE(text.contains("TestAnim"));
}

TEST_F(MCPServerTest, GetAnimationInfoWithSkeletonEntity)
{
if (!canLoadMeshFiles()) { GTEST_SKIP() << "Skipping: entity creation not supported without render window"; }

Ogre::Entity* entity = createAnimatedTestEntity("MCPAnimInfoEntity");
ASSERT_NE(entity, nullptr);

QJsonObject args;
args["entity"] = "MCPAnimInfoEntity";
args["animation"] = "TestAnim";
QJsonObject result = server->callTool("get_animation_info", args);
EXPECT_FALSE(isError(result));
QString text = getResultText(result);
EXPECT_TRUE(text.contains("TestAnim"));
EXPECT_TRUE(text.contains("length") || text.contains("Length"));
}

TEST_F(MCPServerTest, SetAnimationLengthWithSkeletonEntity)
{
if (!canLoadMeshFiles()) { GTEST_SKIP() << "Skipping: entity creation not supported without render window"; }

Ogre::Entity* entity = createAnimatedTestEntity("MCPSetAnimLenEntity");
ASSERT_NE(entity, nullptr);

QJsonObject args;
args["entity"] = "MCPSetAnimLenEntity";
args["animation"] = "TestAnim";
args["length"] = 2.5;
QJsonObject result = server->callTool("set_animation_length", args);
EXPECT_FALSE(isError(result));
EXPECT_TRUE(getResultText(result).contains("2.5") || getResultText(result).contains("length"));
}

TEST_F(MCPServerTest, SetAnimationTimeWithSkeletonEntity)
{
if (!canLoadMeshFiles()) { GTEST_SKIP() << "Skipping: entity creation not supported without render window"; }

Ogre::Entity* entity = createAnimatedTestEntity("MCPSetAnimTimeEntity");
ASSERT_NE(entity, nullptr);

QJsonObject args;
args["entity"] = "MCPSetAnimTimeEntity";
args["animation"] = "TestAnim";
args["time"] = 0.5;
QJsonObject result = server->callTool("set_animation_time", args);
EXPECT_FALSE(isError(result));
}

TEST_F(MCPServerTest, AddKeyframeWithSkeletonEntity)
{
if (!canLoadMeshFiles()) { GTEST_SKIP() << "Skipping: entity creation not supported without render window"; }

Ogre::Entity* entity = createAnimatedTestEntity("MCPAddKfEntity");
ASSERT_NE(entity, nullptr);

QJsonObject args;
args["entity"] = "MCPAddKfEntity";
args["animation"] = "TestAnim";
args["track"] = "Child";
args["time"] = 0.25;
QJsonObject result = server->callTool("add_keyframe", args);
EXPECT_FALSE(isError(result));
}

TEST_F(MCPServerTest, RemoveKeyframeWithSkeletonEntity)
{
if (!canLoadMeshFiles()) { GTEST_SKIP() << "Skipping: entity creation not supported without render window"; }

Ogre::Entity* entity = createAnimatedTestEntity("MCPRemoveKfEntity");
ASSERT_NE(entity, nullptr);

// Remove keyframe at t=0.5
QJsonObject args;
args["entity"] = "MCPRemoveKfEntity";
args["animation"] = "TestAnim";
args["track"] = "Child";
args["time"] = 0.5;
QJsonObject result = server->callTool("remove_keyframe", args);
EXPECT_FALSE(isError(result));
}

TEST_F(MCPServerTest, PlayAnimationWithSkeletonEntity)
{
if (!canLoadMeshFiles()) { GTEST_SKIP() << "Skipping: entity creation not supported without render window"; }

Ogre::Entity* entity = createAnimatedTestEntity("MCPPlayAnimEntity");
ASSERT_NE(entity, nullptr);

QJsonObject args;
args["entity"] = "MCPPlayAnimEntity";
args["animation"] = "TestAnim";
QJsonObject result = server->callTool("play_animation", args);
EXPECT_FALSE(isError(result));
}
108 changes: 107 additions & 1 deletion src/Manager_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -587,11 +587,117 @@ TEST_F(ManagerHeadlessTest, HasAnimationNameNoSkeleton)

auto cubeNode = PrimitiveObject::createCube("AnimNameCube");
ASSERT_FALSE(mgr->getEntities().isEmpty());
Ogre::Entity* entity = mgr->getEntities().last();
// Filter by movable type to avoid casting ManualObjects to Entity
Ogre::Entity* entity = nullptr;
for (auto* obj : mgr->getEntities()) {
if (obj->getMovableType() == "Entity") {
entity = static_cast<Ogre::Entity*>(obj);
}
}
ASSERT_NE(entity, nullptr);

// Primitives have no skeleton, so hasAnimationName should return false
EXPECT_FALSE(mgr->hasAnimationName(entity, "Walk"));
EXPECT_FALSE(mgr->hasAnimationName(entity, ""));

mgr->destroySceneNode(cubeNode);
}

// --- In-memory entity tests ---

TEST_F(ManagerHeadlessTest, CreateInMemoryEntity)
{
if (!canLoadMeshFiles()) { GTEST_SKIP() << "Skipping: entity creation not supported without render window"; }
auto* mgr = Manager::getSingletonPtr();
ASSERT_NE(mgr, nullptr);

auto mesh = createInMemoryTriangleMesh("MgrInMemTriangle");
auto* sceneMgr = mgr->getSceneMgr();
auto* node = mgr->addSceneNode("MgrInMemNode");
auto* entity = sceneMgr->createEntity("MgrInMemEntity", mesh);
node->attachObject(entity);

EXPECT_TRUE(mgr->hasSceneNode("MgrInMemNode"));
EXPECT_FALSE(mgr->getEntities().isEmpty());
}

TEST_F(ManagerHeadlessTest, CreateInMemorySkeletonEntity)
{
if (!canLoadMeshFiles()) { GTEST_SKIP() << "Skipping: entity creation not supported without render window"; }
auto* mgr = Manager::getSingletonPtr();
ASSERT_NE(mgr, nullptr);

auto mesh = createInMemorySkeletonMesh("MgrSkelMesh");
auto* sceneMgr = mgr->getSceneMgr();
auto* node = mgr->addSceneNode("MgrSkelNode");
auto* entity = sceneMgr->createEntity("MgrSkelEntity", mesh);
node->attachObject(entity);

EXPECT_TRUE(entity->hasSkeleton());
EXPECT_EQ(entity->getSkeleton()->getNumBones(), 2u);
}
Comment on lines +636 to +638

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

head -n 670 src/Manager_test.cpp | tail -n 45

Repository: fernandotonon/QtMeshEditor

Length of output: 1680


🏁 Script executed:

rg -n "EXPECT_TRUE\(entity->has(Skeleton|AnimationState)" src/Manager_test.cpp -A 2 -B 1

Repository: fernandotonon/QtMeshEditor

Length of output: 363


Use ASSERT_* for skeleton/animation preconditions before dereference.

At lines 636–637, EXPECT_TRUE(entity->hasSkeleton()) is immediately followed by EXPECT_EQ(entity->getSkeleton()->getNumBones(), 2u). Similarly, at lines 649–652, EXPECT_TRUE(entity->hasSkeleton()) and EXPECT_TRUE(entity->hasAnimationState("TestAnim")) precede a direct call to entity->getAnimationState("TestAnim"). If these expectations fail, the test continues and dereferences potentially invalid pointers, causing a crash rather than a clean assertion failure. Use ASSERT_TRUE or ASSERT_NE for preconditions that guard against null or invalid pointer dereference.

Proposed fix
-    EXPECT_TRUE(entity->hasSkeleton());
-    EXPECT_EQ(entity->getSkeleton()->getNumBones(), 2u);
+    ASSERT_TRUE(entity->hasSkeleton());
+    ASSERT_NE(entity->getSkeleton(), nullptr);
+    EXPECT_EQ(entity->getSkeleton()->getNumBones(), 2u);
...
-    EXPECT_TRUE(entity->hasSkeleton());
-    EXPECT_TRUE(entity->hasAnimationState("TestAnim"));
-
-    auto* state = entity->getAnimationState("TestAnim");
+    ASSERT_TRUE(entity->hasSkeleton());
+    ASSERT_TRUE(entity->hasAnimationState("TestAnim"));
+    auto* state = entity->getAnimationState("TestAnim");
+    ASSERT_NE(state, nullptr);
     EXPECT_NEAR(state->getLength(), 1.0f, 0.01f);

Also applies to: 649–653

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Manager_test.cpp` around lines 636 - 638, The test uses EXPECT_TRUE for
preconditions that are immediately followed by dereferences
(entity->hasSkeleton() before entity->getSkeleton()->getNumBones() and
entity->hasAnimationState("TestAnim") before
entity->getAnimationState("TestAnim")), which can lead to null-pointer
dereference on failure; change those EXPECT_* checks to ASSERT_* (e.g.,
ASSERT_TRUE(entity->hasSkeleton()) or ASSERT_NE(entity->getSkeleton(), nullptr)
and ASSERT_TRUE(entity->hasAnimationState("TestAnim")) or
ASSERT_NE(entity->getAnimationState("TestAnim"), nullptr)) so the test aborts on
failure and avoids dereferencing invalid pointers for getSkeleton(),
getNumBones(), hasAnimationState(), and getAnimationState("TestAnim").


TEST_F(ManagerHeadlessTest, CreateAnimatedEntity)
{
if (!canLoadMeshFiles()) { GTEST_SKIP() << "Skipping: entity creation not supported without render window"; }
auto* mgr = Manager::getSingletonPtr();
ASSERT_NE(mgr, nullptr);

auto* entity = createAnimatedTestEntity("MgrAnimEntity");
ASSERT_NE(entity, nullptr);

EXPECT_TRUE(entity->hasSkeleton());
EXPECT_TRUE(entity->hasAnimationState("TestAnim"));

auto* state = entity->getAnimationState("TestAnim");
EXPECT_NEAR(state->getLength(), 1.0f, 0.01f);
}

TEST_F(ManagerHeadlessTest, HasAnimationNameWithSkeleton)
{
if (!canLoadMeshFiles()) { GTEST_SKIP() << "Skipping: entity creation not supported without render window"; }
auto* mgr = Manager::getSingletonPtr();
ASSERT_NE(mgr, nullptr);

auto* entity = createAnimatedTestEntity("MgrAnimNameEntity");
ASSERT_NE(entity, nullptr);

EXPECT_TRUE(mgr->hasAnimationName(entity, "TestAnim"));
EXPECT_FALSE(mgr->hasAnimationName(entity, "NonExistentAnim"));
}

TEST_F(ManagerHeadlessTest, DestroyInMemoryEntity)
{
if (!canLoadMeshFiles()) { GTEST_SKIP() << "Skipping: entity creation not supported without render window"; }
auto* mgr = Manager::getSingletonPtr();
ASSERT_NE(mgr, nullptr);

auto mesh = createInMemoryTriangleMesh("MgrDestroyTriangle");
auto* sceneMgr = mgr->getSceneMgr();
auto* node = mgr->addSceneNode("MgrDestroyNode");
auto* entity = sceneMgr->createEntity("MgrDestroyEntity", mesh);
node->attachObject(entity);

ASSERT_TRUE(mgr->hasSceneNode("MgrDestroyNode"));
mgr->destroySceneNode("MgrDestroyNode");
EXPECT_FALSE(mgr->hasSceneNode("MgrDestroyNode"));
}

TEST_F(ManagerHeadlessTest, SceneNodeParenting)
{
auto* mgr = Manager::getSingletonPtr();
ASSERT_NE(mgr, nullptr);

auto* parent = mgr->addSceneNode("ParentNode");
ASSERT_NE(parent, nullptr);

auto* child = mgr->getSceneMgr()->getRootSceneNode()->createChildSceneNode("ChildNode");
ASSERT_NE(child, nullptr);

// Move child under parent
mgr->getSceneMgr()->getRootSceneNode()->removeChild(child);
parent->addChild(child);

EXPECT_EQ(child->getParentSceneNode(), parent);
EXPECT_EQ(parent->numChildren(), 1u);
}
Loading
Loading