Adding hooks to catch MRs that lack unit tests

I’m quite new to GitLab & please accept my apologies if I’ve posted this in the wrong area.

I’ve been tasked with investigating whether we can add something to our MR approval process to highlight whether proposed Merge Requests (MRs), which are C++ based code, contain any new unit tests or modifications to existing tests. The overall aim is to remind developers and approvers that they need to think about unit testing.

Ideally a small script would run and detect the presence of additional tests or changes (I accept that there’s a limit to how much can be done here) and display a warning on the MR if they weren’t detected.

An addition step, if possible, would be to block the MR until either, changes were pushed that meet the criteria, or an (extra) field is completed explaining why unit testing is not appropriate for this change. We accept that this is not foolproof but are hoping to use this as part of a bigger push for more test coverage.

We can easily write a script in, say, Python to check for unit tests in the commit(s), but what I don’t know is whether/how we can hook this into the GitLab MR process and whether GitLab is extensible enough for us to achieve the additional step above. Any thoughts? Can this be done, and if so, how would I go about it?

This should absolutely be achievable.

Are you currently using GitLab CI? This will allow you to run your python script and return a non-0 exit code if you’re not happy.

The result will be a failed pipeline which makes the merge button red but still allows merging out of the box I believe.

How does that sound?

Thanks @leetickett, yes, we are using CI, and I’d suspected that this might be the best way of achieving what I’m looking for. The main unknown for me (I don’t know GitLab CI very well so am trying to get up to speed as quickly as possible) is whether/how we implement an override on the UT. I don’t want developers to be forced to add UTs just for the sake of it.

Ideally, I’d like to block the MR unless either a UT is added or one of the approvers (not the developer) ‘marks’ the MR as not needing UTs (e.g. an MR which just adds missing documentation or reformats some code)

By ‘marking’, this could be checking a custom box on the UI or adding free-text in a custom field (though this would probably be a fair bit of work and I’m not sure that GitLab is extensible enough to allow this kind of customisation. It could equally just be achieved by adding some agreed text into a comment (i.e. “NO TEST NEEDED”) which would override the block. This would probably be far easier to achieve but would mean that this pipeline would need to be per-comment, rather than per-commit/push.

I don’t know if we can, in one repo, have a pipeline for MRs that gets triggered per commit/push and a separate one that gets triggered per comment. Do you know if that’s possible?

It’s always tricky when there are 10 ways of achieving something, trying to figure out the “best” way.

How about if your python script makes an API call to check for the note you mention (from one of a number of pre-approved users perhaps?)

So if no unit tests have been added and the pipeline has failed, manage A can add a comment/note “Approved change, no unit tests required”. Then someone can just retry the relevant job… this time it will see the relevant note and pass?

Just remember to implement paging when retrieving the MR notes.

Wdyt?

That sounds like a good plan @leetickett, thanks. I can imagine there might be some pushback about having to manually retry, but to be honest, if we get that far, we’re 90% there anyway. I’ll give it a go. Thanks again.

You could create a webhook to retry the job (or potentially easier to re-run the entire pipeline), but it seems like maybe more effort than it’s worth.

1 Like