chore(test): improve test coverage

[Rolv-Apneseth]

Continuing work on #117

Sorry, I've been away for a while, but here is a quick PR trying to improve the test coverage. I included those 2 tests from #91.

Happy to add more tests if you have suggestions.

I've also gone ahead and broken up api/check.rs into multiple files since it was over 1k lines long (all imports should be the same though).

[Rolv-Apneseth]

On a side-note, I've noticed that it's not possible to create an api::Client with custom hostname and port, since those fields are private. What do you think of adding some builder methods to allow modifying an api::Client? Or do you prefer a constructor where all the fields need to be specified?

And how about a ping method on api::Client as well? It already exists for the CLI, but it seems useful to have for both.

[codspeed-hq]

CodSpeed Performance Report

Merging #136 will not alter performance

_{Comparing Rolv-Apneseth:test_coverage (cac026d) with v3 (ea24862)}

Summary

✅ 6 untouched benchmarks

[jeertmans]

Hi @Rolv-Apneseth, thanks for your PR and the late review, I missed the notification.

On a side-note, I've noticed that it's not possible to create an api::Client with custom hostname and port, since those fields are private. What do you think of adding some builder methods to allow modifying an api::Client? Or do you prefer a constructor where all the fields need to be specified?

Good remark! I don't even remember if there was a good reason for this or not... I'd say it makes sense to include a way to create the api::Clien with something else than default(). We don't really use a builder-like pattern in the library, so I'd rather go for something similar to api::server::ServerClient.

Maybe I am recently getting too distracted from this project, but in fact api::Client doesn't seem to be used, and is a simplified version of api::ServerClient, and does not exist on the main branch. Maybe something happened when refactoring the library.

[Rolv-Apneseth]

Maybe I am recently getting too distracted from this project, but in fact api::Client doesn't seem to be used, and is a simplified version of api::ServerClient, and does not exist on the main branch. Maybe something happened when refactoring the library.

To my knowledge, it's been like that since I came into the mix, so I've left them as separate. Want me to just remove it altogether? The example in the readme also just uses the ServerClient. Though it would be nice to have a way to define the hostname and port for that one too (without needing to use env vars).

[jeertmans]

Maybe I am recently getting too distracted from this project, but in fact api::Client doesn't seem to be used, and is a simplified version of api::ServerClient, and does not exist on the main branch. Maybe something happened when refactoring the library.

To my knowledge, it's been like that since I came into the mix, so I've left them as separate. Want me to just remove it altogether? The example in the readme also just uses the ServerClient. Though it would be nice to have a way to define the hostname and port for that one too (without needing to use env vars).

Yeah, but this is probably remanent from some in-progress work that never got reviewed well enough by me...

[Rolv-Apneseth]

Yeah, but this is probably remanent from some in-progress work that never got reviewed well enough by me...

Fair enough, leave it for now then?

[jeertmans]

Yeah, but this is probably remanent from some in-progress work that never got reviewed well enough by me...

Fair enough, leave it for now then?

I'd rather delete it, and add it back in the future if we really need it, otherwise the same "issue" will occur in the future, and we will likely not know why it's there.

[Rolv-Apneseth]

I'd rather delete it, and add it back in the future if we really need it

Removed that now. Also added a couple more tests, but let me know if you want anything else

[Rolv-Apneseth]

By the way, the last of the failing pre-commit tests for that lint code task is codepsell, not sure what you want to do about it:

 codespell................................................................Failed
- hook id: codespell
- exit code: 65

CHANGELOG.md:318: controler ==> controller
README.md:81: smal ==> small
README.md:81: smal ==> small
README.md:98: smal ==> small
README.md:98: smal ==> small
README.md:164: smal ==> small
README.md:164: smal ==> small

[jeertmans]

By the way, the last of the failing pre-commit tests for that lint code task is codepsell, not sure what you want to do about it:

 codespell................................................................Failed
- hook id: codespell
- exit code: 65

CHANGELOG.md:318: controler ==> controller
README.md:81: smal ==> small
README.md:81: smal ==> small
README.md:98: smal ==> small
README.md:98: smal ==> small
README.md:164: smal ==> small
README.md:164: smal ==> small

We can probably use inline ignore instructions, see here https://github.com/codespell-project/codespell?tab=readme-ov-file#inline-ignore.

What do you think?

[Rolv-Apneseth]

By the way, the last of the failing pre-commit tests for that lint code task is codepsell, not sure what you want to do about it:

 codespell................................................................Failed
- hook id: codespell
- exit code: 65

CHANGELOG.md:318: controler ==> controller
README.md:81: smal ==> small
README.md:81: smal ==> small
README.md:98: smal ==> small
README.md:98: smal ==> small
README.md:164: smal ==> small
README.md:164: smal ==> small

We can probably use inline ignore instructions, see here https://github.com/codespell-project/codespell?tab=readme-ov-file#inline-ignore.

What do you think?

Not sure how to do comments in Markdown but I can look into it when I have a moment

Edit: can probably just use html comments but yeah I'll see how it works for codespell

[Rolv-Apneseth]

Looks like that worked.

This one OK to merge?

[jeertmans]

Looks great, thank you so much @Rolv-Apneseth!!