Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only fetch unresolved issues #177

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

mtughan
Copy link

@mtughan mtughan commented Jul 24, 2024

When fetching issues from SonarQube, we should only fetch unresolved issues. This will avoid showing issues which were discovered in previous runs with the same "PR" ID but have since been resolved and are no longer issues.

Testing done

Tested manually in our local environment.

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

@mtughan mtughan marked this pull request as draft July 24, 2024 19:06
@mtughan
Copy link
Author

mtughan commented Jul 24, 2024

Actually, this PR may still have some issues. Working through them now. Please do not merge yet.

Copy link
Contributor

@reda-alaoui reda-alaoui left a comment

Choose a reason for hiding this comment

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

Please add at least one automated test

@mtughan mtughan force-pushed the fetch-unresolved-issues-only branch from ed23ada to 4f06e7b Compare August 21, 2024 16:55
@mtughan mtughan marked this pull request as ready for review August 21, 2024 17:26
@mtughan
Copy link
Author

mtughan commented Aug 21, 2024

@reda-alaoui, this has been fully tested locally now and resolves our issue nicely. I've also added an automated test to verify the requests that are being sent to SonarQube. Please review when you are able.

@reda-alaoui
Copy link
Contributor

@mtughan , unit tests have low value in this case. Please write an integration test instead.

@mtughan mtughan marked this pull request as draft September 4, 2024 15:45
@mtughan
Copy link
Author

mtughan commented Sep 4, 2024

@mtughan , unit tests have low value in this case. Please write an integration test instead.

Testing an integration test now. Will request review again when I've confirmed it as working (am having issues running it locally).

@mtughan mtughan force-pushed the fetch-unresolved-issues-only branch 5 times, most recently from cf6f65c to 4b63ea9 Compare September 5, 2024 15:15
@mtughan
Copy link
Author

mtughan commented Sep 5, 2024

@reda-alaoui, PullRequestAnalysisTest doesn't seem to be getting executed in the PR runs, either on Linux or on Windows. Any idea why that would be?

12:48:56  [INFO] Running org.jenkinsci.plugins.sonargerrit.sonar.pull_request_analysis.PullRequestAnalysisTest
12:50:12  [INFO] Tests run: 0, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 75.834 s - in org.jenkinsci.plugins.sonargerrit.sonar.pull_request_analysis.PullRequestAnalysisTest

@reda-alaoui
Copy link
Contributor

@mtughan , I am currently investigating this.

@reda-alaoui
Copy link
Contributor

@mtughan please rebase on master to retrieve d12b988 .

@mtughan
Copy link
Author

mtughan commented Sep 17, 2024

@mtughan please rebase on master to retrieve d12b988 .

Will do, thanks.

UPDATE: Yes, it appears to be fixed now. I will troubleshoot and get the unit test working now.

@mtughan mtughan force-pushed the fetch-unresolved-issues-only branch 3 times, most recently from 85ea4cf to 1e14fc8 Compare September 18, 2024 12:28
When fetching issues from SonarQube, we should only fetch unresolved
issues. This will avoid showing issues which were discovered in previous
runs with the same "PR" ID but have since been resolved and are no
longer issues.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants