-
-
Notifications
You must be signed in to change notification settings - Fork 166
Add GitLab code quality reporting. #1878
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
Add GitLab code quality reporting. #1878
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this great addition!
Left a couple of comments and we are good to go.
I will ask you also update the docs and add information about new option and config setting, see examples:
- https://infection.github.io/guide/command-line-options.html#logger-github
- https://infection.github.io/guide/usage.html
Repository to update: https://github.com/infection/site/blob/master/src/guide/command-line-options.md
Do you have any public examples of how this feature works on GitLab already?
@@ -327,6 +327,7 @@ private function retrieveLogs(Logs $logs, string $configDir, ?bool $useGitHubLog | |||
self::pathToAbsolute($logs->getHtmlLogFilePath(), $configDir), | |||
self::pathToAbsolute($logs->getSummaryLogFilePath(), $configDir), | |||
self::pathToAbsolute($logs->getJsonLogFilePath(), $configDir), | |||
self::pathToAbsolute($logs->getGitlabLogFilePath(), $configDir), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we also need --logger-gitlab=<path>
or something like this, to
- make it consistent with
--logger-github
- to have option and config file parity, where this feature can be enabled either from option (
--logger-gitlab
) or from config file by setting"github": "/path/to/file.json"
Related to #141
Created PR for documentation and added the CLI option as requested. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your work, much appreciated.
Released, https://github.com/infection/infection/releases/tag/0.27.3 If you have some public repository where I can see this in action, please let me know. Thanks. |
Thank you! This was the first step in working towards including infection in a project workflow, I will circle back with an example once I have it on a public repo. |
Here are a couple reference links on a public GitLab server showing it in action configured to perform full scans on branch CI runs and to only scan modified files for MR's. MR Overview showing the mutation reported as a Code Quality fault: https://git.drupalcode.org/project/s3fs/-/merge_requests/36 |
@cmlara I'm new to GitLab, so probably not understand somthing completely, but do you know why it doesn't work on my very simple minimal example? https://gitlab.com/maks-rafalko/infection-gitlab-integration-example/-/merge_requests/2
Pipeline definition: https://gitlab.com/maks-rafalko/infection-gitlab-integration-example/-/blob/main/.gitlab-ci.yml#L25-30 |
Unfortunately the "See findings in merge request diff view" does require an Ultimate account which is why it works
On https://gitlab.com/maks-rafalko/infection-gitlab-integration-example/-/merge_requests/2 from the download icon they are in the "infection:archive" and "code_quality:archive" download options (GitLab batches all the artifacts into a single download) the filename can be changed by adding an "expose_as" key https://docs.gitlab.com/ee/ci/yaml/#artifactsexpose_as
On quick glance it does not look like it is a configuration fault in your gitlab-ci.yml. I'm going to have to load up a lab to debug to see if there is a fault in how I implemented the logger plugin. |
Interesting: When I add on the HTML logger it appears to works: ./vendor/bin/infection --logger-gitlab=infection-gitlab.json --logger-html=/dev/null I'm not very familiar with infection's internals, any thoughts on what might cause this? I don't see this on another project, maybe its a dependency related fault? |
there is a bug, fix is here: #1882
I'm pretty sure that when Infection is used only with |
Now it works on my sample project! https://gitlab.com/maks-rafalko/infection-gitlab-integration-example/-/merge_requests/2 |
Ah I missed a location when adding the feature, thank you for finding and creating the fix. To followup on how I failed to reproduce earlier: Looking back at my command logs it appears I was testing with the -s flag that would also be a triggering option. This does appear to be in line with the fix you provided. |
This PR:
Fixes #1877
Adds a new logger format that outputs the GitLab(Code Climate) code quality format.