Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_category_factory: Free Block nodes #93

Merged
merged 1 commit into from
Jun 28, 2024
Merged

test_category_factory: Free Block nodes #93

merged 1 commit into from
Jun 28, 2024

Conversation

dbnicholson
Copy link
Member

Otherwise the test prints warnings about a pile of orphans.

https://phabricator.endlessm.com/T35520

Otherwise the test prints warnings about a pile of orphans.

https://phabricator.endlessm.com/T35520
Copy link
Contributor

@wnbaum wnbaum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok nice! I thought for some reason the CategoryFactory was leaking blocks in the plugin, but it was just the test not cleaning up. Looks good to me!

@dbnicholson
Copy link
Member Author

Ah ok nice! I thought for some reason the CategoryFactory was leaking blocks in the plugin, but it was just the test not cleaning up. Looks good to me!

I definitely thought so, too. However, I think that's actually OK because the Blocks are always transferred into the pickers block list. Later they get cleaned up. However, as noted here, the orphan printing is a feature of GUT. You wouldn't see that in normal operation. You can get the same information from Godot, but it's less clear if we're causing issues or not since it's not nearly as isolated as a single test function. I'm looking at that a bit more. This was just the easy case.

@wnbaum
Copy link
Contributor

wnbaum commented Jun 27, 2024

Yep, as long as there's no nodes that aren't attached to the scene tree I think we're good! I think that when the main panel gets freed on plugin exit, all of the child nodes get freed as well. So as long as we keep all of the blocks we create attached to the picker (or canvas etc.), there should be no leaking.

@wjt
Copy link
Member

wjt commented Jun 28, 2024

@wnbaum feel free to merge PRs you have positively approved.

@wjt wjt merged commit d2f305f into main Jun 28, 2024
2 checks passed
@wjt wjt deleted the T35520-test-orphans branch June 28, 2024 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants