[gui] reenable fit panel testing and fix nullptr access#21882
[gui] reenable fit panel testing and fix nullptr access#21882ferdymercury wants to merge 10 commits intoroot-project:masterfrom
Conversation
Test Results 22 files 22 suites 3d 7h 50m 19s ⏱️ Results for commit a7898cf. ♻️ This comment has been updated with latest results. |
|
I checkout your branch and try to run test.
|
Yes, same here, I had deactivated (commented) locally those old tests for the moment and was focusing on whether it was running at all without crashing.
Weird, where? I see a different behavior on Linux. It just crashes when deleting the TGTooltip at shutdown for me.
Same here, that's what I would say "expected", a lot of errors in batch mode of not being able to load icons, but no crashes. |
I mean when I run compiled executable.
This is most challenging part - try to understand if there double deletion or just wrong pointer released. |
|
I probably found the reason why it crashes. Also side problem is that in Fixing this two problems makes test running for me |
|
First step is #21907 @ferdymercury After such two steps we will be able to enable this and may be other GUI tests in CI |
|
After #21907 is merged you can try rebase your code and see if now fit panel test can be enabled. |
|
Please remove all changes in gui classes from this PR. And at the end of UnitTesting.cxx one should have: And please check |
| result += MakeTest("TestTree1D.........", &FitEditorUnitTesting::TestTree1D); | ||
|
|
||
| result += MakeTest("TestTree2D.........", &FitEditorUnitTesting::TestTree2D); | ||
| // result += MakeTest("TestTree2D.........", &FitEditorUnitTesting::TestTree2D); |
There was a problem hiding this comment.
Remove commented-out lines
There was a problem hiding this comment.
I was thinking of coming back / reenabling these failing tests once we get the current test up and running on all platforms.
(It just needs the definition of a missing function or functor gausNd)
There was a problem hiding this comment.
Then please mark them with a comment explaining that
|
We have several platforms failing. For Windows probably we should force batch mode. One need to add But several linux and mac platforms need to be investigated deeper |
|
So adding Gives: But it's weird, we have |
I also saw this error on Linux, which I think is why the script sometimes crashes with free or munmap_chunk error. |
|
OK, got it, it's the |
|
BTW, the error is also here in non-batch mode. Can you check on Linux (or any non-Windows OS)? |
|
Can you try the following: |
|
Sorry, I push my commit into wrong origin. |
|
I run on Windows |
as suggested by linev, from root-project#21891
by bellenot
Co-authored-by: Sergey Linev <S.Linev@gsi.de>
|
We're still getting the free-invalid-pointer on Unix |
There are only two platforms failing - ubuntu2404 and mac14 - which is already grate. |
Indeed :) It's great. |
Co-authored-by: Sergey Linev <S.Linev@gsi.de>
|
Did you manage to reproduce failures? But I have other idea about ROOT gui testing. Since a while I using So root application will recognize display and will run in normal GUI mode. |
I also see 100% reproducible stack-smashing when I run in interpreted mode rather than compiled mode. (Batch Or Non-Batch). Valgrind says: I seldom also see those munmap_chunk or free invalid pointer failures locally when in compiled mode. Not very often, it's a bit random. It seems that it only happens if the program is fast (goes away if I put debugging points). If I recall correctly, it was in the same place, when deleting the TGToolTip that then calls ~TGGC With valgrind ROOT build Release Mode. I get: so something might be off when initializing TQConnection ? xvfb-run: I also use that for my application, so sounds good. |
|
@linev my proposal would be to merge this 'as is' now (squashed). Disabling the 1D tree test seems to avoid the free/munmap/stack smashing issue in the fit editor. One can then later go back into this TTree issue interaction with the TFitEditor, it's marked as TODO. |
Yes, it sounds reasonable. Lets wait for 6.40 branching and will continue. |
Yep.
also fails with whereas |
This Pull request:
Changes or fixes:
enables old test based on Makefiles by migrating to CMake
step towards #9090
Checklist: