Fix memleaks in unittests
ClosedPublic

Authored by buschinski on Oct 9 2018, 9:03 AM.

Details

Summary

Fix memory leaks in unittests to make it easier to find real memory leaks in the tested code.

Fixed tests:

  • test_projectmodel
  • test_areaoperation
  • test_controller
  • test_toolviewtoolbar
  • test_viewactivation
NOTE: this does not fix all memory leaks in all unittests, this is just s start to keep the changes small
Test Plan
  • cmake -DCMAKE_BUILD_TYPE=Debug -DECM_ENABLE_SANITIZERS='address' -DBUILD_TESTING=ON ..
  • make test

Many tests will fail because of LSAN (leak sanitizer), which is enabled (at least here) by default with ASAN.

Diff Detail

Repository
R32 KDevelop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
buschinski created this revision.Oct 9 2018, 9:03 AM
Restricted Application added a project: KDevelop. · View Herald TranscriptOct 9 2018, 9:04 AM
Restricted Application added a subscriber: kdevelop-devel. · View Herald Transcript
buschinski requested review of this revision.Oct 9 2018, 9:04 AM
kfunk accepted this revision.Oct 9 2018, 9:11 AM
kfunk added a subscriber: kfunk.

Nice!

Can be committed this way as well, but I'd research using QScopedPoiner in some places instead.

kdevplatform/project/tests/test_projectmodel.cpp
325

Here and below (where you use delete ...): Try to use QScopedPointer instead of raw pointers -- that way you don't need to cleanup yourself at the end of the block?

And it also cleans up memory even if we bail out at any of the QCOMPARE or QVERIFY stmts.

This revision is now accepted and ready to land.Oct 9 2018, 9:11 AM
buschinski added inline comments.Oct 9 2018, 10:08 AM
kdevplatform/project/tests/test_projectmodel.cpp
325

Good hint! Will do :)

buschinski updated this revision to Diff 43218.Oct 9 2018, 1:22 PM

Use QScopedPointer where appropriate

Sorry, forgot to push it. Should it go to 5.3 or master?

brauch added a subscriber: brauch.Oct 21 2018, 3:28 PM

Can go to 5.3 in my opinion. Thank you!

kfunk added a comment.Oct 22 2018, 6:39 AM

Many tests will fail because of LSAN (leak sanitizer), which is enabled (at least here) by default with ASAN.

Because I've just read this, note that this behavior can be disabled by doing:

export ASAN_OPTIONS=detect_leaks=0

... before running the tests. Sometimes it's just impossible to get all leaks fixed, because of thirdparty deps, etc..

This revision was automatically updated to reflect the committed changes.