RFR: 1627: Add tests for bot factories [v2]

[Zhao Song]

On Wed, 12 Oct 2022 17:17:39 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

> I still think that using the already existing TestBotFactory is the way to go. Here are my reasons:
> 
> 1. Adding another test implementation doing something very similar to what we already have is generally bad from a future maintenance perspective.
> 2. Having to open up the production class `BotRunnerConfiguration` for subclassing, just to be able to create a test class is sometimes a necessary pattern, but should generally be avoided if possible.
> 3. There is no need to create a `RepositoryHost` and `IssueHost` to instantiate `TestRepository` or `TestIssue`, you can just null those fields using the public constructor. We also don't need a local `Repository` instance (if we fix the constructor in `TestRepository` to stop calling `::url`).
> 4. I don't want these unit tests to need JSON definitions of forges and issue trackers. Especially if those need to fake actual github and jira configurations. That doesn't add any value to the tests, but make these tests depend on how those entities are configured. The scope of the tests in question is to verify the behavior of the specific `*BotFactory` class, not parsing any other parts of a configuration file.
> 5. Needing to create the `HostedRepository` and `IssueProject` instances programmatically is actually a feature here. As an example, it allows us to verify that the factory picks up the correct `RemoteRepository`, as the JSON does not allow us to define specific repositories, just a host.
> 6. Having a test implementation of `BotConfiguration` being used in these tests is a good thing. It makes it easier to customize specific behavior in a test to check corner cases. This is what unit tests are for. Granted, the current implementation is an inline class in TestBotFactory, but that can be refactored if needed. As I mentioned in an earlier comment, if we had access to a mock framework here, I would prefer mocking the `BotConfiguration` completely and just call the `*BotFactory::create` directly, but as we don't have such a dependency present in this project, using the existing test implementation classes is the next best thing.

Thanks for the detailed explanation, I think it makes sense now and  will use `TestBotFactory`.

-------------

PR: https://git.openjdk.org/skara/pull/1393