ci: add unit tests

[SimeonEhrig]

see documentation

[szabo137]

Is it really necessary to replace the whole .gitlab-ci.yml, maybe you could leave at least the linter stage in place.

[SimeonEhrig]

Does the formatter job only check's if it correct formatted? I'm a little bit confused, because the execution is restricted to pull request to the branch dev and main.

[szabo137]

Indeed, the formatter checks, if the code is correctly formatted, i.e. Blue style. The idea behind the trigger rules was, that only well-formatted PR should go through so that eventually the codebase stays in one format.

[SimeonEhrig]

Okay. The rules will not work anymore, because of the mirror feature, it does not open a MR. It simply creates new branch in the upstream repo and push a commit. But I think the rules are not required.

[szabo137]

ok, understood, then I suggest removing the lines (or in this case replacing the whole file, like originally intended), and opening an issue for the missing linter stage.

[SimeonEhrig]

@szabo137 I added the formatter. It works in general. But the can-fail status is not display in GitHub. Instead it shows all fine.
Test-Commit: b156f0b
CI Pipeline: https://gitlab.com/hzdr/qedjl-project/QEDbase-jl/-/pipelines/975875974

I'm not sure, if we want to remove the allow_failure: true flag.

[szabo137]

Oh, that is a pity. Maybe it would be more convenient to move the linter stage to the latest position, after all other tests are passed, and don't allow the linter to fail. Then the whole pipeline fails on the linter and the report icon in GitHub becomes red, isn't it?

[SimeonEhrig]

Normally we run the formatter at the first, because than you don't need to rerun the expensive tests. Also, normally you have a failing formatter job one time and afterwards you configure and automatic formatter job (e.g. in the IDE).

[szabo137]

I do agree with this statement. Then I suggest leaving the linter stage at the first position, but we need to remark the "exclamation mark job" in a review guide in the docs of QED.jl.

[SimeonEhrig]

Do you mean, also keep the allow_failure: true?
We can also run the formatter and the test in the same stage. Then the test are executed but you cannot accidentally merge, if the code is not correct formatted.

[szabo137]

Hmm, I thought it would be more convenient to do linting as a soft check, so it does not annoy you if you push into a draft PR.

[SimeonEhrig]

During developing the PR, I would agree but not if you want to merge it. In this case the code is not correct formatted when it be merged, the next person which modifies the file has a problem. Ether he also format parts, which he does not modified and this appears in the PR or he needs some extra work to format only the part, which he modified. I had the problem with the cling project in the past.

But I don't see technical solution, which prevent use to merge a PR, if the formatter job failed. We have to check the pipeline manually each time, if the job passed, before we hit the merge button.

[szabo137]

I am not happy with this solution. Maybe it would be better, if we use Workflows from GitHub for formatting, instead of the GitLab route.

Nevertheless, to proceed with this PR, I suggest to leave the "you must check the pipeline by yourself" version, but opening an critical issue on this problem in this Repo (and maybe in the repo of the Bot itself) .

[szabo137]

As argued in the comments, this this is not the best solution for the linting stage. However, there are no concerns with the unit-test stages, so I am happy to merge this!

[szabo137]

Ok, one last thing: the latest commit b156f0b1 did not pass the linter stage 🤷‍♂️

[SimeonEhrig]

I removed commit b156f0b1d465859c01adbf6843539e1714fc00e9 because it was only for demonstration. So the current state is, that the formatter runs before unit tests and can fail. If it is fine, merge it.

[szabo137]

Excellent! Merged!