github-action-push-to-another-repository icon indicating copy to clipboard operation
github-action-push-to-another-repository copied to clipboard

Unit testing

Open pwall2222 opened this issue 1 year ago • 6 comments

  • Added unit testing
  • Added separate file for functions
  • Setup of the git submodules is made with:
git submodule update --init
  • Test can be run within the repo with:
./test/bats/bin/bats test/test.bats
  • Uses bats and this are the docs
  • Solves #59 year long issue It's a huge paradigm shift from the one script to rule them all, but the only way to do unit tests is if we have units and functions seem the smallest units of shell scripting appropriate to test.

pwall2222 avatar May 10 '23 09:05 pwall2222

Thanks very much for the MR! I will try to review it soon (weekend or next week). First look is that is good, just need time to read it properly :-)

cpina avatar May 19 '23 09:05 cpina

@cpina You seem very busy, and I don't see a lot of movement in the repository would you be interested on adding me as a mantainer?

pwall2222 avatar Aug 15 '23 18:08 pwall2222

This PR felt into other things that I've been doing.

I will have a better look soon. Thanks again for the PR, I should have acted (merge, feedback, etc.) sooner.

When I had a quick look in May I thought that I was in a catch-22 situation: I don't want to change the code without tests, but to implement the tests, in the PR, the code is changing :-) (and I really want to avoid regressions). I've had a look now and it's not as different as I had initially thought.

Regarding maintainer: I'd like to make the repo more sustainable, so I'll think about it (I have some concerns either way). Let's see first where I get with the PR :-) Thanks for offer your help though (and for the good PR with the needed tests and re-factoring).

cpina avatar Aug 15 '23 22:08 cpina

I will have a better look soon. Thanks again for the PR, I should have acted (merge, feedback, etc.) sooner.

No worries (I always get ghosted on PRs), and forgetting to do things is basically my thing so it's truly alright.

I've had a look now and it's not as different as I had initially thought.

I have purposefully made it as similar as the original as I could just splitting it into functions so it could be tested, further more if you don't like the changes that I propose (that are truly little over at a2e083e01c8506d549d0db9a27115146dfa280aa ) you can just throw away that commit.

Regarding maintainer: I'd like to make the repo more sustainable, so I'll think about it (I have some concerns either way). Let's see first where I get with the PR :-) Thanks for offer your help though (and for the good PR with the needed tests and re-factoring).

It's good to have concerns (trust, communication, etc), but being a lone maintainer is heavy, so even if it's not me (because you don't know me or trust me) you should look into having another maintainer.

I hope I can hear your feedback soon.

pwall2222 avatar Aug 15 '23 22:08 pwall2222

@cpina RFC

pwall2222 avatar Sep 04 '23 21:09 pwall2222

My thoughts at the moment:

  • I really like the approach that you took for the unit tests. If I started the action now I would take this approach!
  • I feel uncomfortable with the "catch 22" situation that generates: change the code to add unit tests, but I wanted to add tests to change the code (in #59 I talk about tests in general, not unit tests to avoid changing the code)
  • Because of #live and #work I prefer to not commit now to make structural changes in the action. I know that is unlikely that will break things, but it's possible. The action is quite used and I want to avoid regressions (and I know that many people use @main instead of the tagged versions). I don't want to have to fix things if I can avoid breaking them (and, when GitHub changed things causing regressions, I fixed them as quickly as I could).
  • I don't think that I should add a maintainer at this point

Side topic: My plans for when I get into this: I want to add an explanation (in the FAQ) in the docs on how to use .gitignore (as mentioned in some issue or MR) to avoid copying / overwriting files. I thought that it was a clever way, and .gitignore has many, many features on includes and excludes. Adding options to the action for this would be endless (exclude a directory, include subdirectories, exclude files..), if it can use .gitignore we can cover many things (which I can if I remember correctly).

I also want to add a list of related github-actions (other actions that serve the same or similar purpose). Not all of them can do everything, and it would be useful to have a bit of a list for when one of them does not fit a use case.

@pwall2222 : an idea is that you fork this one, merge your MR and I add yours (and others!) in the docs. If you do: ping me here and I'll add this section in the docs. I guess that this is probably not your desired way (else you would have already done) but I think that's a way to move forward.

Perhaps, when I have time to write integration tests to ensure that nothing gets broken then I even merge it back? Who knows!

For now I'm happy to leave the MR open because I still find it interesting!

Feel free to get in touch for any comments or thoughts!

cpina avatar Sep 06 '23 22:09 cpina