feat: add build and unit testing CI workflow

[davidbtadokoro]

The project lacks CI workflows that cover the process of checking for compilation problems with cargo build. It also lacks ones that cover unit testing with cargo test.

Considering that there is no point in unit testing the state of the repo if it fails in building, add a single workflow that (sequentially) checks for compilation problems, and runs the unit tests suites.

Note that this workflow is heavily inspired by the recently added format_and_lint.yml.

Closes: #10

[davidbtadokoro]

@OJarrisonn, @lorenzoberts, and @th-duvanel, I would really appreciate your input whenever you have the time :pray:

[OJarrisonn]

The logic of testing only if building succeeds makes sense to me. Just wondering if it is worth to add --no-fail-fast to the unit testing step, so the testing won't stop after the first failure

[lorenzoberts]

LGTM @davidbtadokoro! Regarding the --no-fail-fast mentioned by @OJarrisonn, I have mixed opinions on it. On one hand, it would help the PR's author know all the tests that have failed and avoid extra pushes before resolving them all. Also, since the repo is public, we don't need to worry about GitHub Actions billing, so it would not be a big deal to increase the average time for CI runs. On the other hand, maybe that's not CI's responsibility, since the authors should run the tests themselves before pushing. Besides that, as the project (and consequently the tests) grows, it would take more time to find out that there's a problem in your commit.

[davidbtadokoro]

Hey guys, and thanks a lot for putting time into helping with the review of this change :smile:

@OJarrisonn:

The logic of testing only if building succeeds makes sense to me. Just wondering if it is worth to add --no-fail-fast to the unit testing step, so the testing won't stop after the first failure

@lorenzoberts:

LGTM @davidbtadokoro! Regarding the --no-fail-fast mentioned by @OJarrisonn, I have mixed opinions on it. On one hand, it would help the PR's author know all the tests that have failed and avoid extra pushes before resolving them all. Also, since the repo is public, we don't need to worry about GitHub Actions billing, so it would not be a big deal to increase the average time for CI runs. On the other hand, maybe that's not CI's responsibility, since the authors should run the tests themselves before pushing. Besides that, as the project (and consequently the tests) grows, it would take more time to find out that there's a problem in your commit.

To tell the truth, I was really divided into both approaches. At first, what @OJarrisonn said made a lot of sense to me, but what @lorenzoberts brought to the discussion made me swing back.

Personally, I am more to the opinion of making the CI "complete but minimal". With fail-fast, the build and test workflow will completely catchall compilation problems and the test fails while running the minimal number of actions. This results in less time for CI runs overall.

Of course, if the contributor spams the CI (like mentioned by @lorenzoberts), this won't be the case, but considering that the unit testing suite alone will grow a lot in the future (hopefully), I am more prone to maintaining the workflow like it is.

I will wait a couple of days before greenlighting the merge, so feel free to weigh-in more about this and any other matter.

Thanks again for the time and dedication :pray:

[OJarrisonn]

Think @lorenzoberts approaches is best. Just think we could add a warning, if the testing suit fails tell the user to "run cargo test --idk before opening a PR", just a friendly reminder

[th-duvanel]

Hey, sorry for the delay folks! Didn't have much time these days to read the PR calmly.

Briefly, I agree with @lorenzoberts point and @OJarrisonn last comment. Looks good to me. The first testing should be commiter's responsability, so, a reminder would be nice.

[davidbtadokoro]

Hey guys, thanks again for the attention :smile:

@OJarrisonn:

Think @lorenzoberts approaches is best. Just think we could add a warning, if the testing suit fails tell the user to "run cargo test --idk before opening a PR", just a friendly reminder

Nice!!!

@th-duvanel:

Hey, sorry for the delay folks! Didn't have much time these days to read the PR calmly.

No worries :laughing:

Briefly, I agree with @lorenzoberts point and @OJarrisonn last comment. Looks good to me. The first testing should be commiter's responsability, so, a reminder would be nice.

This @th-duvanel comment basically summarizes my opinion on it. @OJarrisonn final input nailed it! Going to implement this idea.

Thank you so much for the interaction guys :1st_place_medal:

[davidbtadokoro]

Hey guys, I've updated the PR based on what we discussed. It took a little longer than I expected as I was fighting against Bash to print a nice message :grimacing:

When the unit test fails, the message below is thrown

Tell me your opinions on it. If you all are cool with the state of it, I think we can merge it and put this workflow into production.

[lorenzoberts]

Nice ✅