Fix codequality for merge request pipeline (!54620) · Merge requests · GitLab.org / GitLab

Max Orefice requested to merge mo-fix-codequality-merge-request-pipeline into master Feb 18, 2021

Ref:

This feature is behind a feature flag called: codequality_backend_comparison.

What does this MR do?

This MR implements the work done in !44648 (merged) but this time we do it on the backend.

Summary

This merge request adds the ability to reference the pipeline run on the target_sha of a merge request pipeline. The grouptesting recently moved the codequality comparison logic to the backend.

This solution is basically replicating what we did to cleaning up our own MergeRequest widget for report comparison on the frontend but this time we do it for the backend.

A Diagram

graph TD
  A["Commit A (master)"] --> B["Commit B (master, A+B, BASE_PIPELINE)"]
  B --> C["Commit C (master, A+B+C)"]
  B --> F["Commit F (feature, A+B+F)"]
  C --> D["Commit D (master, A+B+C+D)"]
  F --> G["Commit Z (Ephemeral merge commit, A+B+C+D+F, HEAD_PIPELINE)"]
  D --> E["Commit E (master, A+B+C+D+E)"]
  D--> G

This method exposes a pipeline run on commit D from this diagram, if it exists.

Screenshots

Currently & flag off

Notice the second violation Method 'drew' has 19 arguments (exceeds 4 allowed). This method was added to the target branch (master) after the merge request was opened, but remains in the merged result of this merge request.

After switching the flag on

The base_pipeline link has changed, and the artifact is fetched from a job in the more recent pipeline on master, where this violation does exist:

The violation no longer appears in this list, because it's in the base and no longer considered introduced.

Does this MR meet the acceptance criteria?

Conformity

[-] Changelog entry
[-] Documentation (if required)
Code review guidelines
[-] Merge request performance guidelines
Style guides
[-] Database guides
[-] Separation of EE specific content

Availability and Testing

[-] Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process.
[-] Tested in all supported browsers
[-] Informed Infrastructure department of a default or new setting change, if applicable per definition of done

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

[-] Label as security and @ mention @gitlab-com/gl-security/appsec
[-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
[-] Security reports checked/validated by a reviewer from the AppSec team

Fix codequality for merge request pipeline