[gui] reenable fit panel testing and fix nullptr access by ferdymercury · Pull Request #21882 · root-project/root

ferdymercury · 2026-04-10T09:15:31Z

This Pull request:

Changes or fixes:

initialize to null instead

Fixes #21881

Checklist:

tested changes locally
updated the docs (if necessary)

github-actions · 2026-04-10T18:37:56Z

Test Results

22 files 22 suites 3d 9h 8m 4s ⏱️
3 834 tests 3 832 ✅ 1 💤 1 ❌
76 575 runs 76 554 ✅ 18 💤 3 ❌

For more details on these failures, see this check.

Results for commit 6baefa7.

♻️ This comment has been updated with latest results.

linev · 2026-04-13T05:30:22Z

   if (values) {
      fValues = *values;
-      fContext = gVirtualX->CreateGC(gVirtualX->GetDefaultRootWindow(), values);
+      if (gVirtualX)


Checking of gVirtualX does not make big sense - it always present, also in batch mode

It was null when i checked with the debugger. Note that TVirtualX::Instance can still return nullptr when on batch

For changes in GUI classes we need ok from Bertrand @bellenot.
Can we combine this PR with enabling of gui test: #21891
Then we directly can see if our changes works/fails?

And finally we will be able to decide if such extra checks in GUI classes allows to use them in batch GUI tests.

linev · 2026-04-13T15:20:02Z

I checkout your branch and try to run test.
There are many issues with normal GUI mode.

Last two tests TestTree2D and TestTreeND do not work for me - while fit panel does not have entries like "gaus2d" or "gausND".
Also running test in batch possible only with root -b --web=off UnitTesting.cxx. Produced test executable crashed much earlier because TGClient::Instance() returns nullptr
When run inside root, one sees lot of errors about missing images.

ferdymercury · 2026-04-13T19:30:07Z

Last two tests TestTree2D and TestTreeND do not work for me - while fit panel does not have entries like "gaus2d" or "gausND".

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.

Also running test in batch possible only with root -b --web=off UnitTesting.cxx. Produced test executable crashed much earlier because TGClient::Instance() returns nullptr

Weird, where? I see a different behavior on Linux. It just crashes when deleting the TGTooltip at shutdown for me.

When run inside root, one sees lot of errors about missing images.

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.

linev · 2026-04-14T05:53:16Z

Weird, where? I see a different behavior on Linux. It just crashes when deleting the TGTooltip at shutdown for me.

I mean when I run compiled executable.

When run inside root, one sees lot of errors about missing images.

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.

This is most challenging part - try to understand if there double deletion or just wrong pointer released.
Because of lot of errors I cannot exclude incomplete initialization of some GUI classes

linev · 2026-04-14T06:40:24Z

@ferdymercury @bellenot

I probably found the reason why it crashes.
Seems to be it is wrongly set flag TGLabel::fHasOwnFont.
In batch it is incorrect and TGLabel destructor tries to free TGGC object which does not belong to the label.

Also side problem is that in TGGC::TGGC(GCValues_t *values) constructor ref counter not always set correctly.

Fixing this two problems makes test running for me

linev · 2026-04-14T08:19:22Z

First step is #21907

@ferdymercury
Can you extract your changes in TASImage to separate PR.

After such two steps we will be able to enable this and may be other GUI tests in CI

ferdymercury · 2026-04-14T13:16:15Z

+   if (gVirtualX)
+      gVirtualX->ChangeWindowAttributes(fId, &wattr);


Probably these changes are no longer needed since I was trying out different things and you already found the culprit in TGGC

linev · 2026-04-15T06:15:46Z

@ferdymercury

After #21907 is merged you can try rebase your code and see if now fit panel test can be enabled.

linev · 2026-04-15T08:21:51Z

@ferdymercury

Please remove all changes in gui classes from this PR.

And at the end of UnitTesting.cxx one should have:

int UnitTesting()
{
   gROOT->SetWebDisplay("off");

   FitEditorUnitTesting fUT;

   return fUT.UnitTesting();
}

// The main function. It is VERY important that it is run using the
// TApplication.
int main(int argc, char** argv)
{
   TApplication theApp("App",&argc,argv);

   // force creation of client
   if (!TGClient::Instance())
      new TGClient();

   int ret = UnitTesting();

   theApp.Terminate();

   return ret;
}

And please check TestTree1D. It fails while reference values for fir parameters do not match.

as suggested by linev, from root-project#21891

by Linev

silverweed · 2026-04-15T14:27:35Z

      result += MakeTest("TestTree1D.........", &FitEditorUnitTesting::TestTree1D);

-      result += MakeTest("TestTree2D.........", &FitEditorUnitTesting::TestTree2D);
+      // result += MakeTest("TestTree2D.........", &FitEditorUnitTesting::TestTree2D);


Remove commented-out lines

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)

linev · 2026-04-15T14:31:42Z

We have several platforms failing.

For Windows probably we should force batch mode. One need to add -b flag when start executable.

But several linux and mac platforms need to be investigated deeper

ferdymercury mentioned this pull request Apr 10, 2026

[gui,asimage] crash when opening xpm icon in batch mode #21881

Closed

1 task

ferdymercury changed the title ~~[asimage] prevent pointing to invalid memory~~ [gui,asimage] prevent pointing to invalid memory Apr 10, 2026

linev reviewed Apr 10, 2026

View reviewed changes

Comment thread graf2d/asimage/src/TASImage.cxx Outdated

ferdymercury added the pr:squash on merge label Apr 10, 2026

ferdymercury mentioned this pull request Apr 12, 2026

[gui] migrate old Makefiles to CMake and reenable non-covered tests #21891

Closed

devajithvs assigned linev Apr 13, 2026

linev reviewed Apr 13, 2026

View reviewed changes

ferdymercury marked this pull request as ready for review April 13, 2026 08:55

ferdymercury requested review from bellenot and couet as code owners April 13, 2026 08:55

linev reviewed Apr 13, 2026

View reviewed changes

Comment thread graf2d/asimage/src/TASImage.cxx Outdated

linev mentioned this pull request Apr 14, 2026

[gui] let use some classes in batch mode #21907

Merged

ferdymercury mentioned this pull request Apr 14, 2026

[asimage] prevent pointing to invalid memory #21908

Merged

ferdymercury force-pushed the patch-20 branch from bfcbe26 to 522f4c3 Compare April 14, 2026 11:47

ferdymercury changed the title ~~[gui,asimage] prevent pointing to invalid memory~~ [gui] reenable fit panel testing and fix nullptr access Apr 14, 2026

ferdymercury commented Apr 14, 2026

View reviewed changes

ferdymercury force-pushed the patch-20 branch from 522f4c3 to 415db4e Compare April 15, 2026 06:23

ferdymercury added 2 commits April 15, 2026 10:32

[gui] migrate fitpanel test from Makefile to CMake and reenable

1661e26

as suggested by linev, from root-project#21891

[gui] fixes UnitTesting and disable some tree tests (failing)

4678062

by Linev

ferdymercury force-pushed the patch-20 branch from f6f6eec to 4678062 Compare April 15, 2026 08:36

Update CMakeLists.txt

d32767c

silverweed reviewed Apr 15, 2026

View reviewed changes

[test] set batch mode for windows

6baefa7

		if (gVirtualX)
		gVirtualX->ChangeWindowAttributes(fId, &wattr);

Conversation

ferdymercury commented Apr 10, 2026

This Pull request:

Changes or fixes:

Checklist:

Uh oh!

Uh oh!

github-actions bot commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

linev Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

ferdymercury Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

linev Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

linev commented Apr 13, 2026

Uh oh!

Uh oh!

ferdymercury commented Apr 13, 2026

Uh oh!

linev commented Apr 14, 2026

Uh oh!

linev commented Apr 14, 2026

Uh oh!

linev commented Apr 14, 2026

Uh oh!

ferdymercury Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

linev commented Apr 15, 2026

Uh oh!

linev commented Apr 15, 2026

Uh oh!

silverweed Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

ferdymercury Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

linev commented Apr 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Apr 10, 2026 •

edited

Loading