[WIP]8267425: Intermittent failure of HonorDeveloperSettingsTest unit test

[arapte]

The same failure was earlier fixed at PR #496, which seems insufficient. The changes in this PR were also tried ealier in the PR #496 but later removed, as they seemed not necessary. Fix is to clean up the test, by removing all children added to root and hiding the stage. The test only fails on the GHA run, if it continues to fail after this fix then we shall skip it till next test sprint.

---

Progress

• [x] Change must not contain extraneous whitespace
• [ ] Commit message must refer to an issue
• [ ] Change must be properly reviewed

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jfx pull/509/head:pull/509
$ git checkout pull/509

Update a local copy of the PR:
$ git checkout pull/509
$ git pull https://git.openjdk.java.net/jfx pull/509/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 509

View PR using the GUI difftool:
$ git pr show -t 509

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jfx/pull/509.diff

[bridgekeeper[bot]]

:wave: Welcome back arapte! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[mlbridge[bot]]

Webrevs

• 00: Full (41465fe6)

[kevinrushforth]

The cleanup looks safe enough, although I still don't think that removing the children is needed.

I took a look at what the resetStyleManager method in the test does, and it seems like clearing StyleManager::cacheContainerMap should have prevented any leftovers from the old scene from being processed the next time around. And yet that's what it looks like is happening. I'd prefer to understand why it is causing a problem rather than just adding more cleanup and hoping that it's enough this time. I'll take a closer look tomorrow.

[kevinrushforth]

I tried to instrument the code earlier this morning to see what is going on, but I couldn't make it fail, even running it in a loop via GitHub actions on Linux.

One thing to note is that the failure suggests some leftover state that has been partially, but not fully cleaned up. Given where the NPE is, there must be a node in the cacheContainerMap of StyleManager with a non-null scene where the root node of that scene is null. However, unless I'm missing something, this should not be possible since a scene with a null root node is an error. Scene::setRoot throws NPE if you attempt to construct a Scene with a null root or if you later set it to null. Very odd.

[bridgekeeper[bot]]

@arapte This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[bridgekeeper[bot]]

@arapte This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.