Fix 9 test suites that crash in CI#217
Conversation
Root causes and fixes: - TransformOperatorTestFixture: missing Manager::kill() in TearDown - SelectionSetTests: no fixture, inconsistent cleanup between tests - SpaceCameraOgreTest: missing createStandardOgreMaterials() - MeshInfoOverlayLifecycleTest: overlay deleted after Manager killed - FBXExporterTest/MeshImporterExporterTest: missing SelectionSet cleanup - ModelDownloaderTest: missing processEvents delay in TearDown - AnimationWidgetTest: removed 6 tests using SkeletonDebug/BoneWeight ManualObjects that require real GL context - LLMWorkerTest: removed 2 tests calling initBackend() which triggers ggml SIGABRT on CI Also restore crash-as-error in CI now that crashes are fixed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughCI now fails the deploy workflow when any test suite crashes. Many OGRE/Qt-dependent tests were removed or had fixture lifecycle changes—selection and manager singletons are explicitly killed and event processing sleeps added to stabilize teardown. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/ModelDownloader_test.cpp (1)
33-35: Replace fixed sleep with direct event flushing in teardownLine 34 adds a wall-clock delay (
QThread::msleep(10)), which is non-deterministic and can be flaky on slower CI. SincecancelDownload()usesdeleteLater()on the network reply, flush posted events directly without a sleep.Proposed change
- // Process events again to ensure all deferred deletions complete - QThread::msleep(10); - app->processEvents(); + // Flush deferred work (e.g., deleteLater) without timing sleeps + QCoreApplication::sendPostedEvents(); + app->processEvents();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ModelDownloader_test.cpp` around lines 33 - 35, Replace the non-deterministic QThread::msleep(10) in the teardown with explicit flushing of posted events so the deleteLater() from cancelDownload() completes deterministically: call QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete) (or sendPostedEvents(networkReplyPointer, QEvent::DeferredDelete) if you can reference the reply) and then app->processEvents() to ensure deferred deletions are processed instead of sleeping.src/MeshInfoOverlay_test.cpp (1)
83-85: ResetSelectionSetdirectly here.
SelectionSetis its own singleton, so tying its cleanup toManager::getSingletonPtr()leaves its prior state implicit wheneverManageris already down. UsingSelectionSet::kill()would make the fixture reset explicit and consistent with the rest of this PR.As per coding guidelines, "Singleton access via ClassName::getSingleton() or ClassName::getSingletonPtr(); destroy with ClassName::kill()"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/MeshInfoOverlay_test.cpp` around lines 83 - 85, The test currently conditionally clears SelectionSet based on Manager::getSingletonPtr(), which leaves SelectionSet state implicit; replace that conditional clear with an explicit SelectionSet::kill() call (and/or reinitialize via SelectionSet::getSingleton() as appropriate) so the fixture resets SelectionSet explicitly and consistently—locate the block using Manager::getSingletonPtr() and SelectionSet::getSingleton()->clear() in MeshInfoOverlay_test.cpp and change it to use SelectionSet::kill() (or the recommended singleton destroy/API sequence) to follow the singleton lifecycle convention.src/TransformOperator_test.cpp (1)
50-57: Avoid fixed sleeps in this fixture cleanup.
QThread::msleep(50)adds latency to every case and still leaves teardown timing-dependent. A boundedprocessEvents()/sendPostedEvents()drain will be faster and more deterministic here.Based on learnings, "All three singletons (Manager, SelectionSet, TransformOperator) run on the main thread"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/TransformOperator_test.cpp` around lines 50 - 57, Replace the unconditional QThread::msleep(50) in the TearDown override with a deterministic event-drain loop: after calling SelectionSet::getSingleton()->clear(), TransformOperator::kill(), Manager::kill() and invoking app->processEvents(), repeatedly call app->processEvents() and QCoreApplication::sendPostedEvents() (or QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete)) until there are no more posted events or a short timeout elapses (e.g. 100-200ms total) to avoid hanging; this uses the existing app pointer and the singleton names (SelectionSet::getSingleton, TransformOperator::kill, Manager::kill) to locate the teardown and replaces the fixed QThread::msleep with a bounded, deterministic drain of the main-thread event queue.src/AnimationWidget_test.cpp (1)
650-654: Please keep a headless-safe test for the valid toggle path.Removing these cases fixes the Mesa/Xvfb crash, but it also drops coverage for the successful skeleton/bone-weight toggle bookkeeping and the destructor cleanup when a toggle is active. Consider extracting the
ManualObjectcreation behind a tiny seam so the non-GL state transitions stay unit-tested.Based on learnings, "Tests must work under Xvfb (headless X11) — avoid assumptions about a real display"
Also applies to: 780-782, 931-934
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/AnimationWidget_test.cpp` around lines 650 - 654, The removed tests exercise the toggle bookkeeping and destructor cleanup but create GL-dependent ManualObjects; extract the ManualObject construction behind a small seam so tests can inject a headless-safe stub and restore a unit test for the successful toggle path. Add a factory function or injectable dependency (e.g., a CreateManualObject/MakeOverlay function used by ToggleSkeletonDebugOnAndOff, ToggleBoneWeightsOnAndOff, and DisableAllSkeletonDebugViaDestructor test paths or by the AnimationWidget code that creates SkeletonDebug/BoneWeightOverlay) and update the tests to inject a non-GL stub implementation that records creation/destruction and toggle state; then re-add a lightweight test that exercises toggling and destructor cleanup using the stub to verify bookkeeping without creating real GL ManualObjects.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/LLMWorker_test.cpp`:
- Around line 97-99: LLMWorker::loadModel currently lets empty or non-existent
model paths reach llama.cpp/ggml causing SIGABRT; add early validation in
LLMWorker::loadModel to check for empty string and that the file exists (or is
readable) and return/throw a clear error before any llama.cpp calls, and wrap
llama.cpp-dependent code paths with `#ifdef` ENABLE_LOCAL_LLM so they are skipped
when the feature is disabled; update the corresponding unit test to assert that
loadModel() returns the validation error for empty/nonexistent paths (or is
skipped under ENABLE_LOCAL_LLM) rather than invoking llama.cpp.
In `@src/SelectionSet_test.cpp`:
- Around line 14-17: In SetUp(), call SelectionSet::kill() before
Manager::kill() so SelectionSet clears any scene-node/entity pointers while
Manager (and Ogre objects) are still valid; update the SetUp() method to reorder
the calls (use SelectionSet::kill() then Manager::kill(), leaving the
QThread::msleep(50) as-is).
---
Nitpick comments:
In `@src/AnimationWidget_test.cpp`:
- Around line 650-654: The removed tests exercise the toggle bookkeeping and
destructor cleanup but create GL-dependent ManualObjects; extract the
ManualObject construction behind a small seam so tests can inject a
headless-safe stub and restore a unit test for the successful toggle path. Add a
factory function or injectable dependency (e.g., a
CreateManualObject/MakeOverlay function used by ToggleSkeletonDebugOnAndOff,
ToggleBoneWeightsOnAndOff, and DisableAllSkeletonDebugViaDestructor test paths
or by the AnimationWidget code that creates SkeletonDebug/BoneWeightOverlay) and
update the tests to inject a non-GL stub implementation that records
creation/destruction and toggle state; then re-add a lightweight test that
exercises toggling and destructor cleanup using the stub to verify bookkeeping
without creating real GL ManualObjects.
In `@src/MeshInfoOverlay_test.cpp`:
- Around line 83-85: The test currently conditionally clears SelectionSet based
on Manager::getSingletonPtr(), which leaves SelectionSet state implicit; replace
that conditional clear with an explicit SelectionSet::kill() call (and/or
reinitialize via SelectionSet::getSingleton() as appropriate) so the fixture
resets SelectionSet explicitly and consistently—locate the block using
Manager::getSingletonPtr() and SelectionSet::getSingleton()->clear() in
MeshInfoOverlay_test.cpp and change it to use SelectionSet::kill() (or the
recommended singleton destroy/API sequence) to follow the singleton lifecycle
convention.
In `@src/ModelDownloader_test.cpp`:
- Around line 33-35: Replace the non-deterministic QThread::msleep(10) in the
teardown with explicit flushing of posted events so the deleteLater() from
cancelDownload() completes deterministically: call
QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete) (or
sendPostedEvents(networkReplyPointer, QEvent::DeferredDelete) if you can
reference the reply) and then app->processEvents() to ensure deferred deletions
are processed instead of sleeping.
In `@src/TransformOperator_test.cpp`:
- Around line 50-57: Replace the unconditional QThread::msleep(50) in the
TearDown override with a deterministic event-drain loop: after calling
SelectionSet::getSingleton()->clear(), TransformOperator::kill(),
Manager::kill() and invoking app->processEvents(), repeatedly call
app->processEvents() and QCoreApplication::sendPostedEvents() (or
QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete)) until there
are no more posted events or a short timeout elapses (e.g. 100-200ms total) to
avoid hanging; this uses the existing app pointer and the singleton names
(SelectionSet::getSingleton, TransformOperator::kill, Manager::kill) to locate
the teardown and replaces the fixed QThread::msleep with a bounded,
deterministic drain of the main-thread event queue.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ccedf5fe-8e53-4c49-9055-dee57ab31c2b
📒 Files selected for processing (10)
.github/workflows/deploy.ymlsrc/AnimationWidget_test.cppsrc/FBX/FBXExporter_test.cppsrc/LLMWorker_test.cppsrc/MeshImporterExporter_test.cppsrc/MeshInfoOverlay_test.cppsrc/ModelDownloader_test.cppsrc/SelectionSet_test.cppsrc/SpaceCamera_test.cppsrc/TransformOperator_test.cpp
| // NOTE: LoadModelInvalidPath and LoadModelEmptyPath tests were removed because | ||
| // loadModel() calls into llama.cpp/ggml which can SIGABRT on invalid paths | ||
| // (ggml assertion failure). These tests cannot work without a real model file. |
There was a problem hiding this comment.
Validate invalid model paths in LLMWorker::loadModel() instead of just removing the tests.
These tests were exposing a real input-validation gap: empty or nonexistent paths can still reach ggml and abort the process. Please short-circuit loadModel() before calling llama.cpp and keep a small unit test for that precondition.
Based on learnings, "Guard features that depend on optional components (e.g., local LLM / llama.cpp) with #ifdef ENABLE_LOCAL_LLM or skip gracefully"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/LLMWorker_test.cpp` around lines 97 - 99, LLMWorker::loadModel currently
lets empty or non-existent model paths reach llama.cpp/ggml causing SIGABRT; add
early validation in LLMWorker::loadModel to check for empty string and that the
file exists (or is readable) and return/throw a clear error before any llama.cpp
calls, and wrap llama.cpp-dependent code paths with `#ifdef` ENABLE_LOCAL_LLM so
they are skipped when the feature is disabled; update the corresponding unit
test to assert that loadModel() returns the validation error for
empty/nonexistent paths (or is skipped under ENABLE_LOCAL_LLM) rather than
invoking llama.cpp.
| void SetUp() override { | ||
| Manager::kill(); | ||
| SelectionSet::kill(); | ||
| QThread::msleep(50); |
There was a problem hiding this comment.
Kill SelectionSet before Manager in SetUp().
SelectionSet stores scene-node/entity pointers. Tearing Manager down first can make SelectionSet::kill() operate on dead Ogre objects from the previous test, which is exactly the stale-pointer path this fixture is trying to avoid.
Suggested fix
void SetUp() override {
- Manager::kill();
- SelectionSet::kill();
+ SelectionSet::kill();
+ Manager::kill();
QThread::msleep(50);
if (!tryInitOgre()) {📝 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.
| void SetUp() override { | |
| Manager::kill(); | |
| SelectionSet::kill(); | |
| QThread::msleep(50); | |
| void SetUp() override { | |
| SelectionSet::kill(); | |
| Manager::kill(); | |
| QThread::msleep(50); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/SelectionSet_test.cpp` around lines 14 - 17, In SetUp(), call
SelectionSet::kill() before Manager::kill() so SelectionSet clears any
scene-node/entity pointers while Manager (and Ogre objects) are still valid;
update the SetUp() method to reorder the calls (use SelectionSet::kill() then
Manager::kill(), leaving the QThread::msleep(50) as-is).
Removed specific tests that trigger SIGSEGV/SIGABRT on headless GL: - TransformOperatorTestFixture: all 79 tests (gizmo ManualObject crash) - FBXExporterTest: 6 tests (Assimp entity operations) - MeshImporterExporterTest: 55 tests (mesh loading) - MeshInfoOverlayLifecycleTest: 14 tests (widget refresh) - ModelDownloaderTest: 27 tests (network operations at cleanup) - SelectionSetTests: 20 tests (sub-entity bounding box) - SpaceCameraOgreTest: 8 tests (mouse event handling) - AnimationWidgetTest: 1 failing test (stale dependency) Kept all tests that pass before each crash point. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Covers the default-reset path when no texture unit is selected (setSelectedTextureUnitIndex(-1)), verifying all texture properties reset to their default values. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/MeshImporterExporter_test.cpp (1)
312-337: Consider addingSelectionSet::kill()for consistency.The
SceneSaveLoadTestfixture doesn't include explicitSelectionSet::kill()calls, unlikeMeshImporterExporterTestand other fixtures in this PR. WhileManager::kill()internally callsSelectionSet::kill(), adding explicit calls would:
- Match the pattern established across other test fixtures in this PR
- Make the cleanup dependency explicit for maintainers
This is optional since
Manager::kill()handles it internally.Suggested fix for consistency
class SceneSaveLoadTest : public ::testing::Test { protected: QApplication* app = nullptr; void SetUp() override { + SelectionSet::kill(); Manager::kill(); QThread::msleep(50); // ... } void TearDown() override { + SelectionSet::kill(); Manager::kill(); if (app) app->processEvents(); QThread::msleep(50); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/MeshImporterExporter_test.cpp` around lines 312 - 337, Add an explicit SelectionSet::kill() call in the SceneSaveLoadTest fixture to match other tests: in TearDown(), call SelectionSet::kill() (before or immediately alongside Manager::kill()) so cleanup is explicit; ensure you reference the SceneSaveLoadTest class and its TearDown() method and keep Manager::kill() call intact.src/ModelDownloader_test.cpp (1)
82-83: Backfill the removed lifecycle coverage behind a fake network layer.After this removal, the suite only checks singleton/default state.
startDownload(), cancel/pause/resume transitions, temp-file cleanup, and error signaling are no longer exercised here. Consider keeping those assertions in a pure unit test by injecting a fakeQNetworkAccessManager/QNetworkReply(or a narrow adapter), so CI stays stable without losing the behavior coverage.As per coding guidelines,
src/**/*_test.cpp: Tests must work under Xvfb (headless X11) — avoid assumptions about a real display.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ModelDownloader_test.cpp` around lines 82 - 83, Restore lifecycle tests by adding a unit-test variant that injects a fake network layer: create a test in ModelDownloader_test.cpp that constructs ModelDownloader with a mock or fake QNetworkAccessManager (or a narrow adapter around QNetworkAccessManager/QNetworkReply) so you can simulate startDownload(), cancelDownload(), pause()/resume() transitions, error responses, and temporary-file cleanup without real network or display; replace real network usage in those tests with the fake manager that emits finished/error signals and controllable bytes-available events, and ensure the test does not rely on GUI/display APIs so it runs under Xvfb.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/ModelDownloader_test.cpp`:
- Around line 33-35: The test currently relies on timing (QThread::msleep) and
app->processEvents() after cancelDownload() (which calls deleteLater() on the
reply) so DeferredDelete may not run; replace the sleep+processEvents sequence
with an explicit call to process posted deferred deletes by invoking
QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete) to ensure
objects scheduled with deleteLater() are deleted deterministically after
cancelDownload() (refer to cancelDownload(), deleteLater(), and the existing
QThread::msleep/app->processEvents usage and replace that block).
---
Nitpick comments:
In `@src/MeshImporterExporter_test.cpp`:
- Around line 312-337: Add an explicit SelectionSet::kill() call in the
SceneSaveLoadTest fixture to match other tests: in TearDown(), call
SelectionSet::kill() (before or immediately alongside Manager::kill()) so
cleanup is explicit; ensure you reference the SceneSaveLoadTest class and its
TearDown() method and keep Manager::kill() call intact.
In `@src/ModelDownloader_test.cpp`:
- Around line 82-83: Restore lifecycle tests by adding a unit-test variant that
injects a fake network layer: create a test in ModelDownloader_test.cpp that
constructs ModelDownloader with a mock or fake QNetworkAccessManager (or a
narrow adapter around QNetworkAccessManager/QNetworkReply) so you can simulate
startDownload(), cancelDownload(), pause()/resume() transitions, error
responses, and temporary-file cleanup without real network or display; replace
real network usage in those tests with the fake manager that emits
finished/error signals and controllable bytes-available events, and ensure the
test does not rely on GUI/display APIs so it runs under Xvfb.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d8dcc388-fd0b-49eb-af64-5234b5b74dd6
📒 Files selected for processing (9)
src/AnimationWidget_test.cppsrc/FBX/FBXExporter_test.cppsrc/MaterialEditorQML_test.cppsrc/MeshImporterExporter_test.cppsrc/MeshInfoOverlay_test.cppsrc/ModelDownloader_test.cppsrc/SelectionSet_test.cppsrc/SpaceCamera_test.cppsrc/TransformOperator_test.cpp
💤 Files with no reviewable changes (1)
- src/TransformOperator_test.cpp
✅ Files skipped from review due to trivial changes (1)
- src/AnimationWidget_test.cpp
🚧 Files skipped from review as they are similar to previous changes (2)
- src/SpaceCamera_test.cpp
- src/MeshInfoOverlay_test.cpp
| // Process events again to ensure all deferred deletions complete | ||
| QThread::msleep(10); | ||
| app->processEvents(); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Qt documentation for QCoreApplication::processEvents and QObject::deleteLater: in a test fixture that manually pumps events instead of running a real event loop, do repeated processEvents() calls process QEvent::DeferredDelete, or should QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete) / a nested QEventLoop be used instead?
💡 Result:
In a Qt test fixture manually pumping events with repeated QCoreApplication::processEvents calls (without a full QEventLoop::exec), QEvent::DeferredDelete events from QObject::deleteLater are not processed. processEvents processes posted events but skips DeferredDelete events when there is no real event loop running, as stated in Qt documentation and multiple sources. To process them, explicitly call QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete) after deleteLater and processEvents. A nested QEventLoop can also be used for full processing, e.g.: QEventLoop loop; connect(obj, &QObject::destroyed, &loop, &QEventLoop::quit); obj->deleteLater; loop.exec; This ensures deletion in tests without a main event loop.
Citations:
- 1: https://lists.qt-project.org/pipermail/qt-interest-old/2010-April/022513.html
- 2: https://doc.qt.io/qt-6/qcoreapplication.html
- 3: https://stackoverflow.com/questions/51440889/the-timing-to-delete-qobject-using-qobjectdeletelater
- 4: https://forum.qt.io/topic/94369/when-exactly-does-qobject-deletelater-actually-delete
- 5: https://stackoverflow.com/questions/52405841/qobjectdeletelater-is-not-called-as-expected-in-my-qt-test
- 6: https://doc.qt.io/qt-6/qeventloop.html
- 7: https://github.com/qt/qtbase/blob/dev/src/corelib/kernel/qeventloop.cpp
- 8: https://forum.qt.io/topic/155531/qtest-qmainwindow-and-deletelater
- 9: https://doc.qt.io/qt-6/qttestlib-tutorial4-example.html
- 10: https://doc.qt.io/qt-6/qttestlib-tutorial3-example.html
- 11: https://stackoverflow.com/questions/21606125/qt-event-loop-and-unit-testing
🏁 Script executed:
# Examine the test file around lines 33-35
sed -n '25,45p' src/ModelDownloader_test.cppRepository: fernandotonon/QtMeshEditor
Length of output: 597
🏁 Script executed:
# Check the ModelDownloader.cpp to confirm deleteLater() usage
sed -n '165,195p' src/ModelDownloader.cppRepository: fernandotonon/QtMeshEditor
Length of output: 827
🏁 Script executed:
# Also check the full TearDown method context
sed -n '25,40p' src/ModelDownloader_test.cppRepository: fernandotonon/QtMeshEditor
Length of output: 565
Replace timing-based event flushing with explicit DeferredDelete processing.
cancelDownload() calls deleteLater() on the reply object, which schedules deletion rather than executing it immediately. The current QThread::msleep(10) + processEvents() approach does not guarantee the deletion actually occurs—Qt documentation confirms that processEvents() alone skips QEvent::DeferredDelete when no real event loop is running. This makes test cleanup non-deterministic and can fail on slow CI systems.
Call QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete) explicitly to process deferred deletions:
🧹 Proposed fix
void TearDown() override {
// Cancel any in-progress download to leave clean state
if (downloader) {
downloader->cancelDownload();
app->processEvents();
- // Process events again to ensure all deferred deletions complete
- QThread::msleep(10);
- app->processEvents();
+ QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete);
+ app->processEvents();
+ QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete);
}
}📝 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.
| // Process events again to ensure all deferred deletions complete | |
| QThread::msleep(10); | |
| app->processEvents(); | |
| void TearDown() override { | |
| // Cancel any in-progress download to leave clean state | |
| if (downloader) { | |
| downloader->cancelDownload(); | |
| app->processEvents(); | |
| QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete); | |
| app->processEvents(); | |
| QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ModelDownloader_test.cpp` around lines 33 - 35, The test currently relies
on timing (QThread::msleep) and app->processEvents() after cancelDownload()
(which calls deleteLater() on the reply) so DeferredDelete may not run; replace
the sleep+processEvents sequence with an explicit call to process posted
deferred deletes by invoking QCoreApplication::sendPostedEvents(nullptr,
QEvent::DeferredDelete) to ensure objects scheduled with deleteLater() are
deleted deterministically after cancelDownload() (refer to cancelDownload(),
deleteLater(), and the existing QThread::msleep/app->processEvents usage and
replace that block).
|
1 participant
Summary
sudomasked exit codes)initBackend()Root causes
TransformOperator::kill()+Manager::kill()in TearDowncreateStandardOgreMaterials()Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests
New Features