-
Notifications
You must be signed in to change notification settings - Fork 212
refactor random crate search, the random view size is configurable and we don't retry any more #1260
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…d we don't retry any more
82a4d20
to
cb78679
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. Unfortunately, the "feeling lucky" search still occasionally fails.
> i=0; while cargo test lucky; do i=$((i+1)); done
...
thread 'web::releases::tests::im_feeling_lucky_with_stars' panicked at '/releases/search?query=&i-am-feeling-lucky=1: expected redirect to /some_random_crate/1.0.0/some_random_crate/, got 404 Not Found', src/test/mod.rs:80:13
> echo $i
62
But it seems a lot better than before, so I'm ok merging this as a temporary measure.
@jyn514 if nothing serious is blocked because this, give me a little more time today in the evening. I'll figure something out. Flaky tests annoy the hell out of me when I stumble on them, I don't want to be the reason for them :D |
re-adding the retries would help for the tests, but that doesn't feel right. It's tricky with that kind of test-setup, the frameworks I'm used to currently isolate the data for single test-cases. |
After digging into 44f98be I discovered the reason of the problems I had with this test-case.
After discussion in the channel we came onto the issue with this test, driven by the test-setup in 44f98be and me relying on a relation between data and the primary key sequence. |
I ran this 145 times in a row without issues, nice job :) |
@jyn514 thanks for reviewing and merging! And sorry that it broke :) |
The test-database has not very many test-crates in the id-space which have > 100 github stars, so the random-crate-search for I'm feeling lucky fails in the tests.
This refactors the random crate search so the major factor for its success can be configured.