GitHub.getUser() mocking

[The-Huginn]

We have a test case, where we want to retrieve some users and then request a review on a pull request.

We are unable to mock GitHub.getUser() method. We retrieve this GitHub instance with GitHub.connectAnonymously(), which connects to the internet and searches real users. However, with injecting authenticated GitHub instance we are not able to mock the method. Even following #399 suggestion. We do not fancy the latter option, as it requires installation ID.

[gsmet]

Could you give us a bit more information about what you're trying to do? Ideally a small reproducer?

[The-Huginn]

For sure, thanks.

Here is the exact scenario we have. This is the code we want to test https://github.com/The-Huginn/wildfly-github-bot/blob/notify-to-pr-review/src/main/java/io/xstefank/wildfly/bot/TriagePullRequestProcessor.java#L82

and this is the test so far https://github.com/The-Huginn/wildfly-github-bot/blob/notify-to-pr-review/src/test/java/io/xstefank/wildfly/bot/PRReviewAssignmentTest.java#L63

[gsmet]

@yrodiere could you have a look next week? Thanks!

[yrodiere]

You should not connect anonymously, as:

1. In production, you are likely to hit GitHub API limits if you run in a shared environment (cloud).
2. In production, your app will already have access to an authenticated client, which you can inject as explained here: https://docs.quarkiverse.io/quarkus-github-app/dev/developer-reference.html#_injecting_a_github_instance

We do not fancy the latter option, as it requires installation ID.

I'm not sure what the problem is exactly.

In production the installation ID is implicit, you never have to mention it anywhere. So it's not a problem.

You only have to mention the installation ID when mocking, in tests. And that installation ID is already in your JSON, you can find it here. So it's not a problem either... I think?

[The-Huginn]

I have tried the following approach

GHUser user = mocks.ghObject(GHUser.class, 1);
Mockito.when(mocks.installationClient(22950279).getUser(ArgumentMatchers.anyString())).thenReturn(user);

but it does not mock getUser method. I.e. the list reviewers contains 2 null elements.

I thought that tests would also need the private key in combination with installation id, but I guess not.

[yrodiere]

but it does not mock getUser method. I.e. the list reviewers contains 2 null elements.

Does your code inject the GitHub instance like in the documentation, or does it still use GitHub.connectAnonymously() (which won't work)?

If that's not the problem, I'll need you to point me to your code where mocking doesn't work.

[The-Huginn]

It should be using injection, i.e. the GitHub instance used is added as method parameter. In debug I can see matching the installation id as well

[The-Huginn]

I have pushed the latest version into my repo, links above are still relevant.

[yrodiere]

@The-Huginn You're setting up your mocks after the call to then(), so after the event gets simulated. It's too late. You must set up your mock in the first call to github().

E.g. this is incorrect:

        given().github(mocks -> {
                    mocks.configFile(RuntimeConstants.CONFIG_FILE_NAME).fromString(wildflyConfigFile);
                    MockedGHPullRequestProcessor.builder(1371642823)
                            .collaborators(Set.of("user1", "user2"))
                            .mock(mocks);
                })
                .when().payloadFromClasspath("/pr-opened.json")
                .event(GHEvent.PULL_REQUEST)
                .then().github(mocks -> {
                    Mockito.verify(mocks.pullRequest(1371642823), Mockito.times(0))
                            .comment(ArgumentMatchers.anyString());
                    GHUser user = mocks.ghObject(GHUser.class, 1);
                    // Too late to set up mocks!
                    Mockito.when(user.getLogin()).thenReturn("user1").thenReturn("user2");
                    Mockito.when(mocks.installationClient(22950279).getUser(ArgumentMatchers.anyString())).thenReturn(user);
                    ArgumentCaptor<List<GHUser>> captor = ArgumentCaptor.forClass(List.class);
                    Mockito.verify(mocks.pullRequest(1371642823)).requestReviewers(captor.capture());
                    Assertions.assertEquals(captor.getValue().size(), 2);
                    MatcherAssert.assertThat(captor.getValue().stream()
                            .map(GHPerson::getLogin)
                            .toList(), Matchers.containsInAnyOrder("user1", "user2"));
                });

This has more chances to work:

        given().github(mocks -> {
                    mocks.configFile(RuntimeConstants.CONFIG_FILE_NAME).fromString(wildflyConfigFile);
                    MockedGHPullRequestProcessor.builder(1371642823)
                            .collaborators(Set.of("user1", "user2"))
                            .mock(mocks);
                    GHUser user = mocks.ghObject(GHUser.class, 1);
                    Mockito.when(user.getLogin()).thenReturn("user1").thenReturn("user2");
                    Mockito.when(mocks.installationClient(22950279).getUser(ArgumentMatchers.anyString())).thenReturn(user);
                })
                .when().payloadFromClasspath("/pr-opened.json")
                .event(GHEvent.PULL_REQUEST)
                .then().github(mocks -> {
                    Mockito.verify(mocks.pullRequest(1371642823), Mockito.times(0))
                            .comment(ArgumentMatchers.anyString());
                    ArgumentCaptor<List<GHUser>> captor = ArgumentCaptor.forClass(List.class);
                    Mockito.verify(mocks.pullRequest(1371642823)).requestReviewers(captor.capture());
                    Assertions.assertEquals(captor.getValue().size(), 2);
                    MatcherAssert.assertThat(captor.getValue().stream()
                            .map(GHPerson::getLogin)
                            .toList(), Matchers.containsInAnyOrder("user1", "user2"));
                });

[The-Huginn]

Oh absolutely, sorry I didn't realize that. Seems to be working as expected thanks!

Previously it didn't work, as I have my own installation app id and I used that instead of the one in the json file so they were mismatched. I guess we can close this issue.

[yrodiere]

No problem. Closing this, then.