zaynt4606 commented on PR #3859: URL: https://github.com/apache/celeborn/pull/3859#issuecomment-6310492953
There is one remaining test-isolation issue in [`StoragePolicyReplicaSuite`](https://github.com/apache/celeborn/blob/e560ac268f41bd920a1c182f09cbcaeee51de2f6/worker/src/test/scala/org/apache/celeborn/service/deploy/worker/storage/storagePolicy/StoragePolicyReplicaSuite.scala#L49): `MemoryManager.initialize(memoryConf)` runs in the suite constructor during ScalaTest discovery. Other worker suites, including `MemoryManagerSuite`, can reset the shared singleton before these tests execute. `MemoryManager.instance()` then returns null; `StoragePolicy` catches the resulting NPE and falls back to `LocalTierWriter`, so the six cases expecting `MemoryTierWriter` fail. The current Maven service jobs on [JDK 8](https://github.com/apache/celeborn/actions/runs/39380893001/job/118623854663), [JDK 11](https://github.com/apache/celeborn/actions/runs/39380893001/job/118623854859), and [JDK 17](https://github.com/apache/celeborn/actions/runs/39380893001/job/118623854691) all show these same failures. I also reproduced them locally on JDK 17 by running `MemoryManagerSuite` before `StoragePolicyReplicaSuite`: 10 tests passed and 6 failed. Could we make this fixture independent of the shared singleton? A scoped Mockito static mock of `MemoryManager.instance()`, returning a mock whose `memoryFileStorageAvailable()` is true and closing the static mock in `finally`, avoids the discovery/execution ordering problem without changing other suites' singleton state. With that local candidate fix, all 21 storage-policy and memory-manager tests passed together, including `MemoryManagerSuite` before `StoragePolicyReplicaSuite`. The production change looks reasonable to me. Once this remaining test issue is addressed and CI is green, we can continue moving this PR forward. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
