pandaapo commented on PR #4716:
URL: https://github.com/apache/eventmesh/pull/4716#issuecomment-1879561146

   > @Pil0tXia @pandaapo So the exception is caused by the fact that the 
`WatchService` is created using try-with-resources, which means the 
`WatchService` is closed prematurely as soon as the try-with-resources block is 
exited.
   > 
   > I don't have so much domain knowledge here, but would [this latest 
commit](https://github.com/apache/eventmesh/pull/4716/commits/7bea5512676dd9972974fcc4814bed2088b41925)
 be a reasonable fix? If the watch service fails to initialize, there is 
nothing to close, so a `RuntimeException` is thrown. If it is created 
successfully, and it is the registration that fails, then when handling the 
exception, we can close the `WatchService`.
   
   The meaning I expressed in point 2 of 
https://github.com/apache/eventmesh/pull/4716#issuecomment-1877365974 is 
consistent with the modification you made here.
   
   > To get around this, I pushed a commit that restructures the test to use 
Mockito to verify that the `onChanged()` method was called for the file under 
test. This correctly tests whether the change to the file was detected or not.
   > 
   > The one quirk is that it seems to take a bit for the `WatchService.take()` 
method to pick up the change to the file - it was about 10s on my machine. I 
added a 15s timeout to the `Mockito.verify()` call to account for this. With 
the timeout configured, the test will pass as soon as the `onChange()` method 
is called - it doesn't need to wait the full 15s if it is called sooner. There 
is the potential for flakiness here, but I'm not sure how else to account for 
that without some other, more in depth restructuring.
   > 
   > Let me know what you think.
   
   I agree with the validation approach you modified.


-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to