Include JavaScript test coverage in coverage badge

What does this MR do?

Include JavaScript coverage in coverage badge.

Why was this MR needed?

Coverage badge only displayed Ruby coverage.

Screenshot

added 1 commit

• 3a44a5df - Fix?

Compare with previous version

added 1 commit

• c7505f55 - Include JavaScript test coverage in coverage badge (!10097 (merged))

Compare with previous version

changed title from WIP: Add badge for frontend coverage to WIP: Include JavaScript test coverage in coverage badge

Resulting combined coverage badge (Ruby and JavaScript):

added 1 commit

• 4175f11f - Include JavaScript test coverage in coverage badge (!10097 (merged))

Compare with previous version

@grzesiek If you got some time, I would like to have your opinion on this. 😃

unmarked as a Work In Progress

So in other words we have 93% backend coverage + 43% frontend coverage.

Although this might be confusing for users, and having two badges might be better, we have no way of showing which coverage is for frontend and which is for backend code (no way to differentiate badge labels), I think we should do it. It will raise awareness of the low frontend code coverage, and this is probably a good thing to do.

Nevertheless this might have some negative effect when users/customers will see badge. What do you think @stanhu?

we have no way of showing which coverage is for frontend and which is for backend code

I'm leaving a link to #29799 (moved) here. 😄

If we can only have one badge, I think it make sense to average them to draw attention to the low frontend test coverage. I would like to see both numbers somewhere, though. How can we do that?

I would like to see both numbers somewhere

@stanhu Somewhere in the README? Otherwise it is displayed on the top right of the corresponding builds:

• JavaScript: https://gitlab.com/winniehell/gitlab-ce/builds/12745813
• Ruby: https://gitlab.com/winniehell/gitlab-ce/builds/12745817

We could also add something like this to the README:

Test Coverage

• Ruby:
• JavaScript:

That would also allow us to link to both coverage reports.

@winniehell Sounds great!

marked as a Work In Progress

added 1 commit

• f2119bbe - Include JavaScript test coverage in coverage badge (!10097 (merged))

Compare with previous version

added 1 commit

• 3e623c6e - Include JavaScript test coverage in coverage badge (!10097 (merged))

Compare with previous version

added 1 commit

• 3985d9ba - Include JavaScript test coverage in coverage badge (!10097 (merged))

Compare with previous version

resolved all discussions

I adjusted the README and left a screenshot of it above. @grzesiek Can you take another look?

assigned to @grzesiek

unmarked as a Work In Progress

assigned to @winniehell

@winniehell ♥️ I left just a single comment!

added 1 commit

• 74a125fa - Include JavaScript test coverage in coverage badge (!10097 (merged))

Compare with previous version

assigned to @grzesiek

added 1 commit

• fe4e839d - Include JavaScript test coverage in coverage badge (!10097 (merged))

Compare with previous version

resolved all discussions

assigned to @winniehell

resolved all discussions

added 1 commit

• 9d951e5f - Include JavaScript test coverage in coverage badge (!10097 (merged))

Compare with previous version

assigned to @grzesiek

@grzesiek I have opened https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1501 for the corresponding EE changes.

approved this merge request

merged

mentioned in commit 8f6608af

@winniehell LGTM 👍 ♥️ Thanks!

Too bad that we are not able to use secret variables in README.md, we would be able to have GITLAB_RELEASE = ee / ce which could help to avoid conflicts in coverage pages links. 🤔

Too bad that we are not able to use secret variables in README.md

@grzesiek Yes, that would be great: #17005 (moved) 😉

@winniehell I wonder if this job name should be now rspec:coverage or something like this 🤔

It also merges coverage generated by spinach tests. Anyway, we might want to choose another name, because this is not the only job that generates coverage now. What do you think @winniehell?

we might want to choose another name

@grzesiek I agree, I just didn't want to change it. 😃 My personal preference would be coverage:ruby.

@grzesiek FYI milestone was missing (it's important to set one so that we know how many Community Contribution we merged for each milestone! :)).

changed milestone to %9.1