Skip to content

Add inline file coverage view for merge request diffs

Fabio Huser requested to merge siemens/gitlab:feat/mr-diff-coverage-visualisation into master

What does this MR do?

This MR adds the capability to collect automated-testing coverage-reports in the Cobertura XML format within GitLab-CI/CD (new artifacts:reports:cobertura reporter; see example .gitlab-ci.yml bellow). If furthermore adds the capability to process this coverage report and use it to visualize all covered/non-covered lines in the merge request diff file view (see picture bellow).

I've attended a conference where Google showed the importance of fast test-coverage feedback towards developer and reviewers, which helps to improve the the overall coverage without enforcement. I've also found issue #3708 (closed), which is open since quite a while and has an ~"Accepting merge requests" label, but it never really got any traction. That's why I decided to come up with a Community contribution.

While this MR is a MVC and still WIP, the basic functionality is already working, which allows to start discussions about the direction of this MR (see Open Topics).

Closes #3708 (closed)

Screenshots

image

# .gitlab-ci.yml
test:
  script:
    - npm install
    - npx nyc --reporter cobertura mocha
  artifacts:
    reports:
      cobertura: coverage/cobertura-coverage.xml

Open Topics

  • UX - Coverage Visualization: I've tried to follow the initial UX mock-ups in #3708 (closed), but I don't think the coverage visualisation is easily understandable. Is this still the direction to-go or can it be improved (e.g. I like the Google visusalization quite a lot)?
    • The original UX mock-ups seem to be outdated, hence a new proposal based on initial UX feedback was created: !21791 (comment 264872218)
  • backend - BE/FE Communication & Scalability: I've used a similar approach to get the coverage information into the frontend-end, as it's the case for the JUnit test reporting: A single (heavily cached) API endpoint is used to lazy-load the data in one batch. I see a couple of issues with that:
    • I'm not really sure how nicely this scales for bigger projects, since the frontend needs to handle quite a lot of data.
    • MR diffs normally only show a sub-set of all project files, hence there is a lot of unused coverage info within the browser.
    • Since the coverage is mapped 1:1 to files, it would be possible to extend the existing file diff endpoint and augment the line information with coverage information, if available.
    • I've ran some performance tests (GDK on a 2015 MacBook Pro) using GitLab as a reference project. The Karma, Jest and RSpec coverage files sum up to 22MB of Cobertura data. Parsing the files take around 15-18 seconds and result in 2.2MB of converged JSON coverage data. Loading all of this data into the browser works fine and the performance of the MR view is still great. However, I now use diff.diff_files to only return the coverage data for changed files, which drastically reduces the data present in the UI. A also switched the loading of the data to a poll based report mechanism (same as unit-test). The performance is now pretty good and the concept scales to huge projects.
  • backend - Coverage Format: This initial implementation uses the Clover XML format for reporting, since it is quite simplistic yet powerful. However, other formats, such as Cobertura XML are more widely used. The MR is already structured in a way to easily allow further coverage reporting format. Is the Clover XML format good enough for now and in-line with the general direction?
    • After playing a bit around and adding some example project, I've noticed that the Clover format is pretty much dead and quite a few frameworks lack support for it. Furthermore, there is neither an official standard nor does the coverage file contain proper relative paths (only absolute). I've therefore already replaced the Clover parser with a Cobertura parser.

TODOs

  • Finalize coverage UI visualization (see Open Points)
    • Improve UX
    • Improve loading behaviour & mechanism
  • Finalize backend/frontend interface (see Open Points)
    • Improve coverage exchange format
    • Improve caching for coverage endpoint
  • Implement file coverage-merge in Cobertura parser
  • Drop pipeline coverage debug endpoint
  • Add test coverage for all features
  • Performance test with large project and coverage data (e.g. gitlab-org/gitlab)

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

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
Edited by 🤖 GitLab Bot 🤖

Merge request reports