Repository navigation
Speed up slow tests - #2019
Merged
thomas-zahner merged 10 commits intoFeb 9, 2026
Merged
Speed up slow tests#2019
Conversation
thomas-zahner
force-pushed
the
speed-up-slow-tests
branch
2 times, most recently
from
February 2, 2026 02:26
6646340 to
521a8e0
Compare
thomas-zahner
commented
Feb 2, 2026
katrinafyi
reviewed
Feb 8, 2026
katrinafyi
left a comment
katrinafyi
left a comment
Member
There was a problem hiding this comment.
looks good. it's a very good idea to disentangle the cache tests.
Remove dependency on previously deleted TEST_INVALID_URLS.html
The test is now running locally without requiring an internet connection
The previous assertion conincidentially matched the timestamp 1769996167
thomas-zahner
force-pushed
the
speed-up-slow-tests
branch
2 times, most recently
from
February 9, 2026 13:53
8fc4097 to
54d7e3b
Compare
The test was previously lying and confusing. Unknown status codes such as 999 are actually cached today. linkedin returned 200 given lychee's user agent.
Co-authored-by: katrinafyi <[email protected]>
thomas-zahner
force-pushed
the
speed-up-slow-tests
branch
from
February 9, 2026 14:00
54d7e3b to
c5f7348
Compare
Merged
mre
added a commit
that referenced
this pull request
Mar 25, 2026
After thinking about this for a while, I think caching errors goes against typical CI workflows. That's because if a link fails due to a network issue, a server outage, or if a previously failing link has been fixed, reading an error from the cache prevents lychee from realizing the link is now working. Right now it will load the broken link from cache, notice that it has no status code associated with it, and fail immediately. It just caches the error again until it expires and gets re-checked. That's quite unintuitive. This commit updates Cache::store and Cache::load to skip CacheStatus::Error. As a result, failures are always rechecked on subsequent runs. We completely sidestep issues with caching transient network errors or missing HTTP status codes. It also updates a CLI test to accurately assert that an invalid 999 response generates an empty cache file, restoring the original logic of the test which was accidentally flipped in a prior PR (#2019) to expect errors to be cached. Fixes #1734. Fixes #1783.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix #1892.
In CI we can see that tests now take about 55 seconds instead of 70 seconds. The CI action spends a lot of time in compiling lychee. This change is mainly an improvement for local development. E.g. on my machine the first stage of
make test(cargo nextest run --all-targets --all-features) now takes about 5 seconds instead of 12 on average.Apart from speeding up the tests I could also improve the reliability.
Some background details
test_lycheecache_exclude_custom_status_codesrequires no internet but was slow because of unnecessary retries.test_require_httpsis using example.com. As it seems really unpractical to make localhost functional with HTTPS and it generally isn't slow I would prefer to keep this test unchanged.Fix the flakiness of the cache tests. They were racing against each other writing and deleting the cache files of each other.