Compare MR code coverage to target branch

It's nice having the coverage percentage with the builds in a merge request, what could be even nicer would be to show the change compared to the target branch; currently to work this out you have to open the target branch, open its head commit, switch to the builds tab then manually calculate the difference.

My suggestion would be to show the percentage point difference, probably coloured based on whether it's positive/negative e.g. if the target branch build is at 45.7% and the MR build is at 47.2% it would display as 47.2% (+1.5%).

mentioned in issue #19410 (moved)

Added ~19173 label

mentioned in issue #26534 (closed)

added feature proposal ~13921 labels

mentioned in issue #28679 (closed)

@dimitrieh What do you think about UX side of this feature proposal?

It's gaining popularity, this could be a nice feature

Proposal

1. In the first iteration, we would look at target_branch (to which we merge) and source_branch (from which we create merge) and compare coverage of found pipelines,
2. We would use SystemNote and push it to Merge Request once the pipeline from either target and source_branch finishes,
3. Let's assume for now that we react only for source_branch pipeline finish and consider that we have coverage for target_branch (it means that pipeline did finish).

Other considerations

1. Should we compare merge request versions?
2. Should we compare against the first pipeline before introduced changeset?

Details

1. From PipelineSuccessWorker call a service (that is derived from MergeRequests::BaseService) to figure out merge requests for given pipeline,
2. Extend SystemNoteService to create merge request note,
3. Extend PipelineSuccessWorker to react for target_branch pipelines.

Solid proposal @ayufan!

mentioned in issue #28748 (closed)

I'd like to mention a few other considerations:

• I think what a user is most interested in is the result of their merge to the target branch. To show that, I think we would need to simulate the merge into master and run coverage against that (if it merges cleanly).
• But, in the interest of shipping something soon, it's perhaps best to compare the coverage specifically for @base_pipeline (which is #pipeline_for(source_branch, diff_base_sha) which from my skimming of the code seems to be the "first pipeline before the introduced changeset" that @ayufan mentions
• We should use the merge base rather than comparing the coverage for the tips of the source branch and the target branch, because it's easy to imagine a scenario where the source branch doesn't touch coverage in any way (e.g. a documentation edit) but the target branch changes before the MR is closed. If we're just comparing coverage for the latest pipelines for each branch, this could make the user think their docs MR somehow affected coverage.
• In the next iteration of this, I think it would be OK to simpyl just move to running coverage on a simulated merge and using that in the math for the UI - yes, the way we calculate percentages would change silently but a) it is becoming more accurate / meaningful, not less, and b) coverage percentage changes are metadata generally interesting at the time of merge and not the most crucial project metadata.

I'll put a MR together for this this week.

mentioned in merge request !10722

@danielwh

Your MR looks nice and simple. We can still improve the code, and I saw that @grzesiek did start commenting on it.

I think what a user is most interested in is the result of their merge to the target branch. To show that, I think we would need to simulate the merge into master and run coverage against that (if it merges cleanly). In the next iteration of this, I think it would be OK to simply just move to running coverage on a simulated merge and using that in the math for the UI - yes, the way we calculate percentages would change silently but a) it is becoming more accurate/meaningful, not less, and b) coverage percentage changes are metadata generally interesting at the time of merge and not the most crucial project metadata.

I wonder how we could implement: coverage on CI merge. Maybe we should first think about CI on merge. We have this issue that is open: Pipelines for Merge Requests and Run build on merged code. Could you try to read these issue and figure out the simplest possible change that would allow us to do that? They also describe a lot of about current architecture of runners, and pipelines.

The problems to figure out:

1. How and where we should track such merge requests?
2. Should pipelines be created on a branch, or created on a merge request?
3. Who should create merge commit? Should it be the runner, or should it be GitLab?
4. What would we do when master becomes updated?
5. What data have we to add to the database to store this information?
6. How it would work with main/fork repositories, currently all jobs are run in the context of fork, not parent?
7. What would be the simplest change in GitLab code that would make it possible?

Could you try to make the proof of concept of that?

mentioned in issue #31653 (closed)

mentioned in issue #4044 (closed)

Let me address your questions one by one with my thoughts. I spent a bit of time going through the docs and codebase, figuring out how the gitlab architecture looks, but it's a pretty big system so I welcome any challenges and clarification.

• How and where we should track such merge requests?
  • Not sure what this means exactly, as I think this should be the process for all merge requests. If you mean how and where do we track pipelines run against merge results, we can filter a MR's pipeline collection on the ref field, as it will match the special merge ref pattern
• Should pipelines be created on a branch, or created on a merge request?
  • IMO pipelines should not be created on a branch until a MR or new commit is pushed to it. That is to say if the head of a ref has the same sha as a pipeline that has already been run from another ref (eg if you branched new-branch off master but did not push new commits) CI is not run. If a user opens a MR with a sha at the tip that matches an existing CI run, we could also show that existing pass/fail data rather than re-running.
• Who should create merge commit? Should it be the runner, or should it be GitLab?
  • GitLab should attempt to merge immediately when the target or source branch changes and create a ref ror it, so that users other than GitLab CI can use it for whatever purposes they need.
  • This merge should have a ref like refs/merge-requests/:id/merge
• What would we do when master becomes updated?
  • update the merge result ref
  • e.g. Travis+GitHub doesn't re-run CI when this ref is updated, but it might be a nice competitive edge / improvement for GitLab to do so
    • with a big project with rapidly updating master, we should update the merge ref as fast as the changes come in, but debounce the CI run to like 15-30 minutes, or just limit merge result pipeline runs to a parallelism of 1 (not sure how this currently works)
• What data have we to add to the database to store this information?
  • We don't need to add a flag on pipelines because we can just compare ref against /refs\/merge-requests\/\d+\/merge/
  • We don't need to add timing for debouncing CI runs because we can compare timestamps / check status
  • Unless I'm missing something we can infer all we need from existing model fields
• How it would work with main/fork repositories, currently all jobs are run in the context of fork, not parent?
  • This is a really important distinction for all CI jobs, related to secure vars etc and is outside the scope of this problem. We do whatever we are doing for all ci jobs on forks when this is shipped. I have my own opinions about this but again, out of scope
• What would be the simplest change in GitLab code that would make it possible?
  • Update the relevant services (I believe MergeRequests::CreateService and GitPushService) to create and update the MR merge ref - if someone has more experience with the relevant services, pls point me in the right direction
  • Update CreatePipelineService and CreatePipelineService to create new pipelines iff:
    • there isn't a pipeline currently running against that merge ref
    • the desired merge is free of conflicts

The first step should be an isolated bit of work that just creates the merge ref when the source or target branch changes. This + the merge_status field in the current MR API is immediately useful to people that want an easier path to integrating this behavior themselves with 3rd party CI, and supports this behavior with GitLab CI in the future.

The first step should be an isolated bit of work that just creates the merge ref when the source or target branch changes. This + the merge_status field in the current MR API is immediately useful to people that want an easier path to integrating this behavior themselves with 3rd party CI, and supports this behavior with GitLab CI in the future.

Could we implement that first step?

What to do:

1. Create a merge result on separate ref, ideally using asynchronous worker,
2. Create a pipeline for merge result,
3. Present pipeline on merge request.

changed milestone to %Backlog

mentioned in merge request !12215

moved to gitlab#4822

Hey!

GitLab is moving all development for both GitLab Community Edition and Enterprise Edition into a single codebase. The current gitlab-ce repository will become a read-only mirror, without any proprietary code. All development is moved to the current gitlab-ee repository, which we will rename to just gitlab in the coming weeks. As part of this migration, issues will be moved to the current gitlab-ee project.

If you have any questions about all of this, please ask them in our dedicated FAQ issue.

https://gitlab.com/gitlab-org/gitlab-ee/issues/13855

You can also find more information here:

• https://gitlab.com/gitlab-org/gitlab-ee/issues/13304
• https://about.gitlab.com/2019/08/23/a-single-codebase-for-gitlab-community-and-enterprise-edition/

Frequently Asked Questions

For an up to date list of questions and answers, please take a look at https://gitlab.com/gitlab-org/gitlab-ee/issues/13855

What will we do with the repository names?

https://gitlab.com/gitlab-org/gitlab-ee/ will become https://gitlab.com/gitlab-org/gitlab, and https://gitlab.com/gitlab-org/gitlab-ce will become https://gitlab.com/gitlab-org/gitlab-foss.

Why rename gitlab-ce to gitlab-foss?

Using "gitlab" and "gitlab-ce" would be confusing, so we decided to rename gitlab-ce to gitlab-foss to make the purpose of this FOSS repository more clear

I created a merge requests for CE, and this got closed. What do I need to do?

You will need to create a merge request at https://gitlab.com/gitlab-org/gitlab-ee/. This link will eventually redirect to https://gitlab.com/gitlab-org/gitlab.

How does the licensing work in this new setup?

Everything in the ee/ directory is proprietary. Everything else is free and open source software. If your merge request does not change anything in the ee/ directory, the process of contributing changes is the same as when using the gitlab-ce repository.

Will you accept merge requests on the gitlab-ce/gitlab-foss project after it has been renamed?

No. Merge requests submitted to this project will be closed automatically.

Will I still be able to view old issues and merge requests in gitlab-ce/gitlab-foss?

Yes.

How will this affect users of GitLab CE using Omnibus?

No changes will be necessary, as the packages built remain the same.

How will this affect users of GitLab CE that build from source?

Once the project has been renamed, you will need to change your Git remotes to use this new URL. GitLab will take care of redirecting Git operations so there is no hard deadline, but we recommend doing this as soon as the projects have been renamed.

Where can I see a timeline of the remaining steps?

https://gitlab.com/gitlab-org/gitlab-ee/issues/13304

Will community contributions submitted to the new "gitlab" repository be available in the new gitlab-foss repository?

Yes, these changes will be synced to gitlab-foss automatically, several times per day.

locked this issue

mentioned in issue gitlab#5331