search

namib-project / dcaf-rs

Implementation of the ACE-OAuth framework.
4 stars 1 forks source link

Update CI pipeline to remove actions-rs #23

Closed pulsastrix closed 3 months ago

pulsastrix commented 3 months ago

As the actions-rs GitHub actions are no longer maintained, this PR replaces those with suitable alternatives.

Strongly inspired by the CI pipeline in namib-project/libcoap-rs and the grcov README

coveralls commented 3 months ago

Coverage Status

coverage: 84.95%. remained the same when pulling 554e607035c9220a68fff6d9552db981dfe82ba0 on update_ci_pipeline into 0f6a7c227c1e7dac7ad669e0fd0b1046ce96b42d on main.

pulsastrix commented 3 months ago

I've also replaced the current coverage tool (grcov) with tarpaulin, although I am unsure if this should be kept in.

With grcov, I had some issues with excluding directories. I got an obscure missing file issue when adding --ignore /* as was previously done by the actions-rs version of grcov, but not adding it causes some non-code files to be included (no idea where the coverage values for those come from?!?), see https://coveralls.io/builds/69151273.

cargo tarpaulin however seems to have a bunch of false negatives and/or positives, see https://coveralls.io/builds/69151472/source?filename=src%2Ftoken%2Fcose%2Fmaced%2Fmod.rs#L208 for instance.

Considering that the coverage numbers seem to be vastly different when comparing the current state of main with the one of this PR, at least one of the two different coverage reporting tools seems to be very off.

Any opinions on this (pinging @falko17)?

falko17 commented 3 months ago

Considering that the coverage numbers seem to be vastly different when comparing the current state of main with the one of this PR, at least one of the two different coverage reporting tools seems to be very off.

Any opinions on this (pinging @falko17)?

Hm, we should investigate more deeply why these numbers are so different (67% vs. 85%, if I interpret this correctly). Given the extensive tests we have in place, I trust the 85% number more, but this could also be too optimistic—we need to take a deeper look into it before we decide on which tool to use.

pulsastrix commented 3 months ago

As tarpaulin seems to have a number of false negatives, I have switched back to grcov and fixed the aforementioned issues.

The line coverage calculation now seems to be fine, the coverage percentage being lower is due to the branch coverage being aggregated with the line coverage, see https://coveralls.io/jobs/149319128.

Should I keep this setting enabled? It can be disabled in https://coveralls.io/github/namib-project/dcaf-rs/settings.

falko17 commented 3 months ago

Should I keep this setting enabled? It can be disabled in https://coveralls.io/github/namib-project/dcaf-rs/settings.

I don't think branch coverage is necessary/useful for this project, so I've disabled the aggregation for now so the number is easier to interpret (and higher :D).