Allow excluding cache based on status code

[dmathieu]

Closes #1400

This introduces an option --cache-exclude-status, which allows specifying a range of HTTP status codes which will be ignored from the cache.

[mre]

Perfect. Great work!

Perhaps the only thing I'd add is an integration test, similar to this: https://github.com/lycheeverse/lychee/blob/eff77d634283610dbdd481db2d9b1d1162cdea84/lychee-bin/tests/cli.rs#L794-L858

Probably it's just a matter of copy-pasting that block and changing the parameter, to test the new flag. Would you be interested in adding that? I could understand if it's outside the scope for now, though, and I'd be fine with merging it like it is as well. Let me know what you prefer.

[dmathieu]

I'm happy to do it, but it'll have to wait for a few days. I can then do it either in this PR, or in a new one.

[mre]

Just tested the changes locally. When I run lychee like below, the cache is empty:

❯❯❯ cargo run -- --verbose --cache https://lychee.cli.rs/
    Finished dev [unoptimized + debuginfo] target(s) in 0.19s
     Running `target/debug/lychee --verbose --cache 'https://lychee.cli.rs/'`
[INFO ] Cache is recent (age: 1m 6s, max age: 1d 0h 0m 0s). Using.
✔ [200] https://lychee.cli.rs/introduction/
✔ [200] https://lychee.cli.rs/#_top
✔ [200] https://lychee.cli.rs/favicon.svg
✔ [200] https://lychee.cli.rs/
✔ [200] https://lychee.cli.rs/_astro/logo.BJx6koUn.svg
✔ [200] https://lychee.cli.rs/sitemap-index.xml
✔ [200] https://lychee.cli.rs/_astro/index.fVW1leCO.css
✔ [200] https://lychee.cli.rs/
✔ [200] https://lychee.cli.rs/
✔ [200] https://lychee.cli.rs/_astro/logo.BJx6koUn_1qY8Fi.svg
✔ [200] https://github.com/lycheeverse/lycheeverse.github.io/edit/master/src/content/docs/index.mdx
✔ [200] https://github.com/lycheeverse/lychee/

🔍 12 Total (in 1s) ✅ 12 OK 🚫 0 Errors
~/C/p/l/lychee ❯❯❯ echo $?
0
~/C/p/l/lychee ❯❯❯ cat .lycheecache
~/C/p/l/lychee ❯❯❯ 

[mre]

I've added the integration test we discussed, and that one seems to work. Something isn't adding up here. 🤔

[dmathieu]

Could that be related to a cache file already existing? (in which case, the issue could be that in the case of running tests with an existing cache, the file isn't written again?)

[dmathieu]

I'm not sure whether there's still something missing in this PR?

[mre]

Sorry for the late response.

The feature doesn't work right now. I tested it by excluding only a single status code, but it excludes everything.

Could that be related to a cache file already existing? (in which case, the issue could be that in the case of running tests with an existing cache, the file isn't written again?)

Nope, tried in a directory without a cache file.

I didn't have time to look at it, but I guess it has something to do with the defaults of StatusCodeSelector. I know we don't use StatusCodeSelector::default for the excluded status codes, but it looks like the defaults get picked up. As a consequence, status codes like 200 get excluded by default when the setting is used, rendering the feature sort of useless.

If you have some time to investigate or test on your side, I would greatly appreciate the effort. :)