search

jmakhack / myanimelist-cli

Minimalistic command line interface for fetching user anime data from MyAnimeList.
https://aur.archlinux.org/packages/mya-git
MIT License
11 stars 15 forks source link

Add support for unit testing #23

Closed TBeeren closed 1 year ago

TBeeren commented 2 years ago

Associated Issue

Closes #13

Implemented Solution

welcome[bot] commented 2 years ago

Thanks for opening this pull request for myanimelist-cli! Please wait shortly for someone to review. https://media1.tenor.com/images/de54dcf0d5723e5c190b36aed008917f/tenor.gif

TBeeren commented 2 years ago

Oh woops, since the fork is on the same repo it overwrited itself. Let me create a new one for " Add contributing and development details to repo #23 ".

This is the PR for the test environment #13 . You're able to test the application now. However, I'm not the best in Makefile. I think you can make this nicer, looking at your makefile skills. For now, the testcases aren't testing the application yet. This is because all your functions are void functions, and you're exiting the program on wrong input. To be able to test the program nicer I should advice to add some mocks or testing functions. 😄

jmakhack commented 2 years ago

@TBeeren, thanks for the contribution! This is definitely a big step forward towards being able to add proper unit testing to this project.

Looks like the build check after the changes is failing as seen here: https://github.com/jmakhack/myanimelist-cli/runs/3982742098?check_suite_focus=true. Can you take a look as to why the build is failing?

TBeeren commented 2 years ago

Thanks a lot @jmakhack! Yes will have a check why the builds are failing! Probably a makefile problem (undefined references). Are you stuck to Makefiles or would you be open to CMake aswel? Really feel like cmake makes live easier for these projects haha.

jmakhack commented 2 years ago

Thanks a lot @jmakhack! Yes will have a check why the builds are failing! Probably a makefile problem (undefined references). Are you stuck to Makefiles or would you be open to CMake aswel? Really feel like cmake makes live easier for these projects haha.

Go for it! I'm open to using CMake for this project.

I would just request that you make sure to also edit .github/workflows/c-cpp.yml and .github/workflows/codeql-analysis.yml to make sure they use the correct commands instead of make all.

TBeeren commented 2 years ago

@jmakhack Will do! Currently a bit busy with my 'normal' work, but will try to do the final tweeks to make this work asap 😄

jmakhack commented 2 years ago

@jmakhack Will do! Currently a bit busy with my 'normal' work, but will try to do the final tweeks to make this work asap 😄

No worries @TBeeren! Take as much time as you need in completing this task. There is no rush. And if it turns out you won't have time to complete this, just let me know and I can work off of what you've already completed.