refactor: libwayshot tests improvements

[Gigas002]

Refactor tests: move all test stuff into tests.rs to not pollute logics code with tests code.
If possible, I'd like to move all tests in completely different module, but rust guys consider this anti-pattern for some reason, so the only sane possible way - to keep tests in libwayshot crate, but in different file, at least.
Also increase test coverage by adding several uncovered areas's tests.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 12.25%. Comparing base (238f1d2) to head (091f44d).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #330      +/-   ##
==========================================
- Coverage   17.53%   12.25%   -5.28%     
==========================================
  Files          20       19       -1     
  Lines        3005     2545     -460     
==========================================
- Hits          527      312     -215     
+ Misses       2478     2233     -245     

Flag	Coverage Δ
libwayshot	16.15% <ø> (-5.89%)	⬇️
wayshot	0.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Gigas002]

Hmm, I wonder why codecov shows drop in coverage...

[Gigas002]

Factual code coverage is up. I guess some computation error. Maybe realted to error.rs: all tests have been moved out of there, and now it's pure enum file.
This PR: https://app.codecov.io/gh/waycrate/wayshot/pull/330/tree/libwayshot/src?dropdown=coverage
Main's last commit: https://app.codecov.io/gh/waycrate/wayshot/commit/238f1d27c3520dfa8ca54b459e23f56c21eef573/tree/libwayshot/src?dropdown=coverage

[Decodetalkers]

No... I am not favor this. Usually, we use mod test under that module, and write tests. And the module name will be always called tests... Why do we need to change the style of unit test?

[Gigas002]

Yeah, that's the matter of taste, I guess. Personally I think it's just:

1. Easier to browse real code, since files are slimmer
2. Easier to browse commits and do review: as soon as you see which file has the changes you understand if its tests or not

I don't understand why rust/go guys prefer writing tests near the normal code. Sometimes you browse through commit history and see something like 'changes xxx.rs' and think "yeah this must be the change I was looking for", but when actually checking you see only test update inside.

[Decodetalkers]

The problem is the module name. I think you can split the unit tests to different files under tests, and make them the same name as the module that the test for

For example

--tests.rs
---- error.rs
-----screencopy.rs

Then it will be acceptable for me

[Gigas002]

Yeah that sounds reasonable. I'll take a look if we can do that without separating tests in completely different crate

[Gigas002]

@Decodetalkers
What do you think of current implementation, would that be acceptable?

[Decodetalkers]

I think they can not be end with test I think

[Gigas002]

I was thinking this would conflict with files naming on above level, but it indeed works fine lol

[Decodetalkers]

I think this pr is acceptable, but maybe we need to hear from other members

[Gigas002]

Sure, I don't mind closing it if there are any objections to this refactor and since it's only matter of taste.
I just personally absolutely hate this style of test pollution in code, but if everyone else is OK with it, or there is something I'm missing I'll be glad to hear out.

[Gigas002]

@Shinyzenith @AndreasBackx
Could you please ACK/NACK this when you have free time?
We're basically discussing the way to handle tests implementation in app, architecture wise.

Current way: like most rust apps, do unit tests in same files where the logic lives
This PR change: move tests out (in the same crate) into different files

[AndreasBackx]

@Gigas002 The changes look fine to me. Though I feel like what would make the most "impact", is if we could dump a few outputs we would currently get from a few different monitor setups. If we could integrate that somehow into the testing framework, we could essentially entirely replicate the flow end to end.

A tool could be created that would dump this information so when someone reports a bug, we could ask them to run the debug CLI of sorts to get the information we need, add it to the testing framework, and then run the tests on it. The files will likely be quite big so it'd be good to used zstd compression or something similar to store them in the repository.