scullionw / dirstat-rs

(fastest?) disk usage cli, similar to windirstat.
MIT License
157 stars 11 forks source link

Added unit tests and Github work flows to run tests and lints #10

Open Mart-Bogdan opened 1 year ago

Mart-Bogdan commented 1 year ago

Lints would post all clippy warnings as annotation to pool request

Mart-Bogdan commented 1 year ago

By the way. It should add tests count as comment to PR, like here: https://github.com/Mart-Bogdan/dirstat-rs/pull/3#issuecomment-1355791986

I guess it is because it is not merged yet. Or because I am not a maintainer.

image

Mart-Bogdan commented 1 year ago

By the way. It should add tests count as comment to PR I guess it is because it is not merged yet. Or because I am not a maintainer.

Oh. That's unfortuante:

Publish Unit Tests Results This action is running on a pull_request event for a fork repository. It cannot do anything useful like creating check runs or pull request comments. To run the action on fork repository pull requests, see https://github.com/EnricoMi/publish-unit-test-result-action/blob/v1.20/README.md#support-fork-repositories-and-dependabot-branches

Mart-Bogdan commented 1 year ago

Hi, @scullionw. I've addressed commets, and removed dead code in actions.

Would there be other comments regarding tests?

Sorry for slow reaction time. I have issues with internet and electricity, and also some chores.

Mart-Bogdan commented 1 year ago

I actually don't like to put actual files into repo, but I wasn't able to come with anything more clever.

Perhaps generating them as random files with predefined seed, hm.

Random files are mostly needed to work with compressed files.

Mart-Bogdan commented 1 year ago

Hello, @scullionw. Happy holidays. Happy new year.

So what about this PR?)

scullionw commented 1 year ago

do all tests pass without the other PR? if not lets mark them as #[should_panic], and then remove them in the other PR, so that each PR by itself is mergeable

Mart-Bogdan commented 1 year ago

do all tests pass without the other PR? if not lets mark them as #[should_panic], and then remove them in the other PR, so that each PR by itself is mergeable

I did not include tests that won't work without other PR. Besides, they won't compile. They are still in separate branch.

Mart-Bogdan commented 1 year ago

Ah. shuch tests currently are marked as #[cfg(not(windows))] hmm.

Mart-Bogdan commented 1 year ago

Hello, @scullionw, regarding integration tests.

I'm thinking. This crate exports library as well as executable. Is it required, and do we need to keep backward compatibility for lib?