contentdb icon indicating copy to clipboard operation
contentdb copied to clipboard

Allow coveralls.io images in the CSP

Open Lazerbeak12345 opened this issue 2 years ago • 6 comments

Problem

A common way of measuring unit test code coverage is to send the coverage results to a third party, such as https://coveralls.io. They provide badges:

Here's one: Coverage Status

and the code:

[![Coverage Status](https://coveralls.io/repos/github/Lazerbeak12345/flow-extras/badge.svg?branch=master)](https://coveralls.io/github/Lazerbeak12345/flow-extras?branch=master)

Right now, when I include this markdown into a CDB page, the image does not appear, and I get this message in the DevTools:

content.minetest.net/:1163 Refused to load the image 'https://coveralls.io/repos/github/Lazerbeak12345/sway-inv/badge.svg?branch=master' because it violates the following Content Security Policy directive: "img-src 'self' data: *.minetest.net secure.gravatar.com *.imgur.com imgur.com *.bitbucket.com bitbucket.com *.github.com github.com *.githubusercontent.com *.gitlab.com gitlab.com *.shields.io *.notabug.org notabug.com".

Solutions

I think adding coveralls.io would not be a bad idea.

Alternatives

Might be able to do it using shields, or an alternative coverage metric provider.

  • https://shields.io/badges/coveralls-branch

Additional context

https://content.minetest.net/packages/lazerbeak12345/sway/ and https://content.minetest.net/packages/lazerbeak12345/flow-extras/ are two packages with this problem (though the first has yet to be approved and can only be seen by moderators, at the time of posting, and me fixing this issue was a request of wsor4035. I wanted to try this before the shields.io approach, since it works a bit better.)

Lazerbeak12345 avatar Sep 24 '23 01:09 Lazerbeak12345

This works as a workaround.

https://shields.io/badges/coveralls-branch

Lazerbeak12345 avatar Sep 30 '23 16:09 Lazerbeak12345

I don't recommend showing code coverage badges (or unit test or ci or other technical badges) on ContentDB as users don't care, that's something better for your repo readme.md and not the ContentDB description

rubenwardy avatar Sep 30 '23 16:09 rubenwardy

You know, that's a good point.

On one hand, a user might understand the significance of unit testing and what code coverage is. If they do, they would appreciate it.

But on the other hand, unless the mod is an API first and foremost, the target audience would be the players, an audience that doesn't necessarily know or care.

In my mind, I see both of these mods as mostly on the API side of that scale, where they would be depended upon by a modpack that adds the full functionality that a user would expect. This uses contentdb as a distribution method for APIs that can be built upon to make a complete experience. I should tag these mods as APIs if I haven't done already.

At the end of the day, it's your software, made for the original developer of MineTest. I'm not going to demand anything of you.

Concerning the mods themselves, if the content moderation team doesn't like the badge, then I simply won't include the badge.

Lazerbeak12345 avatar Sep 30 '23 18:09 Lazerbeak12345

a user might understand the significance of unit testing and what code coverage is

Warning, opinions ahead. Watch your step.

If they really do understand rather than just thinking it might be significant then they'll be reading the code anyway to see if coverage numbers actually means something.

I mean with bad tests one can easily get badge coverage very high and even all the way to 100% with very buggy code and without fixing anything.

For this reason I do not think it really is useful even if they understand. In this case they'd very likely be updating mods directly with git rather than CDB anyway.

S-S-X avatar Jan 20 '24 14:01 S-S-X

It's literally just allowing a domain name though?

Not to mention that it's already possible, though less convenient, to see coverage metrics?

In fact, one of the only badge services allowed even lets you just make a fully custom icon. No need to put in the work to do any tests at all - and one wouldn't need to change which badges cdb allows.

I just feel like your argument was a strawman.

Lazerbeak12345 avatar Jan 20 '24 22:01 Lazerbeak12345

I just feel like your argument was a strawman.

Very much mean this, I've noticed some less tech savvy people look just at how many badges something has and draws conclusions from that. This is why I recommend using those primarily on developer facing interfaces and leave them out from consumer facing interfaces.

I bet there's other just as good marketing strategies for CDB content while giving end users some information that's actually useful as is without extensive tech knowledge.

S-S-X avatar Jan 20 '24 22:01 S-S-X

I don't really want to add too many domains to the CSP, I'd like to remove GH and others in fact.

This PR would allow any domain to be used without privacy issues: #294

Also, worth noting that images can't be shown in the Minetest client, they appear as links

rubenwardy avatar May 24 '24 00:05 rubenwardy