kevinthedang / discord-ollama

Discord Bot that utilizes Ollama to interact with any Large Language Models to talk with users and allow them to host/create their own models.
Creative Commons Attribution 4.0 International
73 stars 8 forks source link

Unit Testing for Code Coverage #58

Closed JT2M0L3Y closed 4 months ago

JT2M0L3Y commented 4 months ago

Added

Notes

JT2M0L3Y commented 4 months ago

The tests look fine. I haven't run them yet, but I hope they work.

URL link for Build is not working as intended as of now. Might fix itself later by running workflows.

The url for build does not work in branches other than master. This is because the build workflow is only triggered upon a push to master (the result of merging this PR).

I ran the existing tests many a time before pushing. Each existing test works with the current code base.

kevinthedang commented 4 months ago

Also I do not believe the version needs to be incremented because we are just introducing tests not a feature (unless you think we should then change it appropriately. @JT2M0L3Y

JT2M0L3Y commented 4 months ago

is there a reason why you're doing __tests__/ instead of tests/?

This is a convention from other NodeJS projects I have seen. However, there is a good chance this is an older styling convention and we could rather use just a folder named /test to model a UNIX-like file system. Would this be preferrable?

However, per this stack overflow post, the __tests__ styling allows any file within such a directory to be considered a test. Meaning, I could modify the naming of files in this directory by dropping the .test. portion.

Is there any way we can Mock the environment variables to reduce dependency on github? I think it's better to allow for isolation.

We could define a distinct, example set of environment variables that are compared during testing instead of using those passed into the Keys object by the workflow.

kevinthedang commented 4 months ago

Might even consider tests with their files but we shouldn't do that prob.

I guess it does not really matter per this discussion. But essentially, if we do not anticipate other "test" folders then we should even move the folder to src/tests but we can do 'tests/. I'm more down for the latter but let me know what you think.