PoiBlackTea commented on PR #1132: URL: https://github.com/apache/yunikorn-core/pull/1132#issuecomment-5378156951
> Yes, I do agree with the cleanup. > > Is test really asserting the intended checks? Doesn't look like. It is more of ensuring IncAllocatedResource() correctness rather than checking the Quota Preemption related values. Isn't it? > > Can you ensure the test is doing the work that it is intended to? It is supposed to ensure Quota Preemption feature correctness even after the restart. > > #1068 had introduced this test. May be you can take a look at it to understand the reason for introducing this test at the first place. Thanks for the guidance @manirajv06 I have updated the test to properly reflect the scheduler recovery and quota preemption verification goals: - Real Recovery Simulation: Uses registerAllocations() instead of manual resource increments to trigger the actual PartitionContext.UpdateAllocation recovery flow. - Accurate Completion Check: Asserts queueA.GetAllocatedResource() to confirm the recovered allocation is fully reflected in the queue. - Direct Preemption State Validation: Directly asserts QuotaPreemptionStartTime from GetPartitionQueueDAOInfo(false) to verify whether the preemption delay timer is scheduled under each test condition, replacing the flawed waitForAllocations(0/1) checks. Please take another look when you have time. Thanks -- 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]
