Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add unit testing framework #363

Open
cjappl opened this issue Mar 10, 2024 · 8 comments
Open

Add unit testing framework #363

cjappl opened this issue Mar 10, 2024 · 8 comments

Comments

@cjappl
Copy link
Collaborator

cjappl commented Mar 10, 2024

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

  • Files with spaces
  • Files with brackets
  • Files not previewing on one operating system

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
Copy link
Collaborator

I think this would be a big improvement.

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.

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
Copy link
Collaborator Author

cjappl commented Mar 11, 2024

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
Copy link
Collaborator

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
Copy link
Owner

wfxr commented Mar 12, 2024

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
Copy link
Collaborator

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
Copy link
Owner

wfxr commented Mar 13, 2024

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
Copy link
Collaborator

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

@sandr01d
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants