wfxr / forgit

:zzz: A utility tool powered by fzf for using git interactively.
MIT License
4.4k stars 137 forks source link

Add unit testing framework #363

Open cjappl opened 6 months ago

cjappl commented 6 months ago

We have had a decent number of regressions and issues between shell/OS versions.

It would be great to set up a unit testing framework and eventually github actions to test that things are staying consistent and prevent further regressions.

Things like

Have all been fixed/broken/rebroken.

@sandr01d found this, which seems pretty promising! https://github.com/kward/shunit2

I also think this may be easier once #326 lands, as it breaks the code into a bunch of small simple functions ripe for unit testing.

sandr01d commented 6 months ago

I think this would be a big improvement.

I also think this may be easier once https://github.com/wfxr/forgit/pull/326 lands, as it breaks the code into a bunch of small simple functions ripe for unit testing.

I agree. E.g. the preview functions are even callable as sub commands, which should make implementing unit tests for them a lot easier.

My idea on how to approach this would be by creating a separate repository that holds the unit tests and files to test on. We could then create an action that checks out both the test repo and this one and run the tests.

cjappl commented 6 months ago

My idea on how to approach this would be by creating a separate repository that holds the unit tests and files to test on. We could then create an action that checks out both the test repo and this one and run the tests.

What are your thoughts on the advantages this would bring? I think one potential downside is just more complexity to manage (user needs to know about the extra repo, or deal with the pain that is git submodules).

carlfriedrich commented 6 months ago

I would strongly suggest to put the unit tests into this repo, so that we can update tests along with the code in a single PR. Updating tests should therefore also get another checkbox in our PR template.

wfxr commented 6 months ago

I would strongly suggest to put the unit tests into this repo, so that we can update tests along with the code in a single PR. Updating tests should therefore also get another checkbox in our PR template.

I also have the same view.

sandr01d commented 6 months ago

What are your thoughts on the advantages this would bring?

The advantage would be to keep the main repo free of clutter. I expect that there would be quite a lot test files (all sort of files with special file names etc.).

I think one potential downside is just more complexity to manage (user needs to know about the extra repo, or deal with the pain that is git submodules).

No, that would all be handled by the action that automatically runs when you push to a PR.

I would strongly suggest to put the unit tests into this repo, so that we can update tests along with the code in a single PR.

That is a good point though, probably best to keep them in the main repo then.

wfxr commented 6 months ago

The advantage would be to keep the main repo free of clutter. I expect that there would be quite a lot test files (all sort of files with special file names etc.).

@sandr01d For unit test, I think we can just test functions like filename parser without executing git commands. Constructing filename strings should be much simpler than creating tons of real files. Although this way may not necessarily cover all scenarios, it is already valuable as long as it can cover the most error-prone areas. What do you think?

sandr01d commented 6 months ago

That sounds like a reasonable start @wfxr. In case we notice regressions in other areas we could still expand our coverage.

sandr01d commented 6 months ago

I think we should include tests against bash 3.2 and the BSD sed that ships with macOS if possible. These are the most common pitfalls for me personally. See e.g. #373