NehanPathan commented on PR #1170: URL: https://github.com/apache/lucenenet/pull/1170#issuecomment-3239239323
> Thanks. Removing `Perform()` wasn't intentional. And thanks for adding the tests. > > Although, I realized that by switching our tests to using async only, we are no longer testing the synchronous methods. I am looking into how to set it up without too much code duplication. I think we pretty much have to duplicate everything in `ReplicationServlet`. The `NewHttpServer()` method overloads can get a bool `async` parameter. > > Then comes the tricky bit. I think the `HttpReplicatorTest` class could be converted into a `HttpReplicationServerTestCase` that is abstract. Then an abstract bool property could be added named `UseAsyncServer`, which would be passed into `NewHttpServer()`. Then we could just make 2 new classes, `AsyncHttpReplicatorTest` and `HttpReplicatorTest` that subclass `HttpReplicationServerTestCase`, each setting the bool value appropriately. > > I am about to turn in for the night, but would be happy to work on it tomorrow if you don't get the chance. @NightOwl888 Thanks — that makes sense. Your approach sounds clean and avoids too much duplication. I’m happy to try setting that up now, unless you’d prefer to handle it yourself since you already have the structure in mind. Either way, I’ll follow your lead — just let me know what you prefer. -- 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: dev-unsubscr...@lucenenet.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
