Skip to content

Add tests for DesktopAssetManager cache and asset events#2812

Open
Dingyi189-eng wants to merge 1 commit into
jMonkeyEngine:masterfrom
Dingyi189-eng:test-asset-manager-cache
Open

Add tests for DesktopAssetManager cache and asset events#2812
Dingyi189-eng wants to merge 1 commit into
jMonkeyEngine:masterfrom
Dingyi189-eng:test-asset-manager-cache

Conversation

@Dingyi189-eng
Copy link
Copy Markdown

Adds unit tests for DesktopAssetManager caching and AssetEventListener callbacks.

  • Verifies cache hits do not fire assetLoaded twice
  • Verifies clearCache() forces a reload
  • Verifies loadAsset(null) fails fast

Run: ./gradlew :jme3-core:test --tests "com.jme3.asset.DesktopAssetManagerCacheTest"

Co-authored-by: Cursor <cursoragent@cursor.com>
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new test class, DesktopAssetManagerCacheTest, to verify caching and AssetEventListener behavior in DesktopAssetManager. A critical issue was identified in the cachedLoadFiresRequestedButNotSecondLoadedEvent test, where Assert.assertNotSame(first, second) is used incorrectly. Since Texture is a shared asset and not cloned upon cache retrieval, both instances should be identical, and the assertion should be updated to Assert.assertSame to prevent the test from failing.

Assert.assertEquals(2, listener.requestCount());
Assert.assertEquals(1, listener.loadedCount());
Assert.assertNotNull(assetManager.getFromCache(textureKey));
Assert.assertNotSame(first, second);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Since Texture is not a CloneableSmartAsset, it is a shared asset and is not cloned upon cache retrieval. Therefore, the first and second loaded textures should be the exact same instance. Using Assert.assertNotSame(first, second) will cause the test to fail. It should be changed to Assert.assertSame(first, second).

Suggested change
Assert.assertNotSame(first, second);
Assert.assertSame(first, second);
References
  1. Issues found in test code should be reported with a reduced priority, at most medium.

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.

1 participant