Skip to content

Conversation

@jeromepochat
Copy link
Contributor

@jeromepochat jeromepochat commented Aug 29, 2024

#396 fixed GitHubAppCredentials owner inference using withOwner method which not considered as a public API.

This uses Connector.lookupScanCredentials from GitHub Branch Source plugin to contextualize the credentials automatically. It removes the direct usage of GitHubAppCredentials.withOwner from github-checks-plugin.

This includes some mocking adjustments to involve the credentials contextualization provided by github-branch-plugin.

Testing done

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

new PluginLogger(j.createTaskListener().getLogger(), "GitHub Checks"),
wireMockRule.baseUrl())
.publish(details);
try (var credentialsMatchers = mockCredentialsMatchers()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Introduced try-with-resource for static mock auto-close. The body is indented without any change.

.contains("endColumn=20")
.contains("annotationLevel=WARNING")
.contains("message='say hello to Jenkins'");
try (var credentialsMatchers = mockCredentialsMatchers()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Introduced try-with-resource for static mock auto-close. The body is indented without any change.

@codecov
Copy link

codecov bot commented Sep 16, 2024

Codecov Report

Attention: Patch coverage is 80.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 71.58%. Comparing base (803edab) to head (3a32e03).
Report is 22 commits behind head on master.

Files with missing lines Patch % Lines
...s/plugins/checks/github/GitHubChecksPublisher.java 60.00% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #398      +/-   ##
============================================
- Coverage     71.80%   71.58%   -0.22%     
- Complexity      162      163       +1     
============================================
  Files            16       16              
  Lines           532      535       +3     
  Branches         51       51              
============================================
+ Hits            382      383       +1     
- Misses          124      126       +2     
  Partials         26       26              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

try (MockedStatic<Connector> connector = mockStatic(Connector.class)) {
connector.when(() -> Connector.connect(anyString(), any(GitHubAppCredentials.class))).thenReturn(gitHub);
try (var credentialsMatchers = mockCredentialsMatchers(); var connector = mockStatic(Connector.class)) {
connector.when(() -> Connector.lookupScanCredentials(any(), any(), any(), any())).thenCallRealMethod();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call real method to involve GitHubAppCredentials contextualization


when(scmFacade.findGitHubSCMSource(job)).thenReturn(Optional.of(source));
when(scmFacade.findGitHubAppCredentials(job, "1")).thenReturn(Optional.of(gitHubAppCredentials));
when(scmFacade.findGitHubAppCredentials(job, "1")).thenCallRealMethod();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call real method to involve GitHubAppCredentials contextualization

@jeromepochat jeromepochat marked this pull request as ready for review September 16, 2024 21:59
@jeromepochat jeromepochat requested a review from a team as a code owner September 16, 2024 21:59
@timja timja added the enhancement New feature or request label Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants