Skip to content
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

Regression in new bisect implementation in cargo-msrv v0.14.0 #288

Closed
foresterre opened this issue Feb 2, 2022 · 0 comments · Fixed by #289
Closed

Regression in new bisect implementation in cargo-msrv v0.14.0 #288

foresterre opened this issue Feb 2, 2022 · 0 comments · Fixed by #289
Milestone

Comments

@foresterre
Copy link
Owner

foresterre commented Feb 2, 2022

    use super::Bisect;
    use crate::check::Check;
    use crate::outcome::{Outcome, SuccessOutcome};
    use crate::reporter::no_output::NoOutput;
    use crate::search_methods::FindMinimalCapableToolchain;
    use crate::semver::Version;
    use crate::toolchain::{OwnedToolchainSpec, ToolchainSpec};
    use crate::{semver, Config, ModeIntent, TResult};
    use rust_releases::Release;

    struct FakeRunner;

    impl Check for FakeRunner {
        fn check(&self, config: &Config, toolchain: &ToolchainSpec) -> TResult<Outcome> {
            Ok(Outcome::Success(SuccessOutcome {
                toolchain_spec: OwnedToolchainSpec::new(toolchain.version(), config.target()),
            }))
        }
    }

    fn fake_config() -> Config<'static> {
        Config::new(ModeIntent::Find, "".to_string())
    }

    #[test]
    fn all_are_valid() {
        let search_space = vec![
            Release::new_stable(semver::Version::new(1, 58, 1)),
            Release::new_stable(semver::Version::new(1, 57, 0)),
            Release::new_stable(semver::Version::new(1, 56, 1)),
        ];

        let bisect = Bisect::new(FakeRunner {});

        let result = bisect
            .find_toolchain(&search_space, &fake_config(), &NoOutput {})
            .unwrap();

        assert_eq!(result.unwrap_version(), Version::new(1, 56, 1));
    }

Actual result: Version::new(1, 57, 0)
Expected result: Version::new(1, 56, 1)

The reason for this result is that the indices left == right which causes the algorithm to stop. In this case, we'd like to go one step further, since we want to go beyond an insertion point.

@foresterre foresterre added this to the v0.14.1 milestone Feb 2, 2022
bors bot added a commit that referenced this issue Feb 2, 2022
289: Fix regression where last release was not tested when last in search space r=foresterre a=foresterre

Fixes #288 

Co-authored-by: Martijn Gribnau <garm@ilumeo.com>
bors bot added a commit that referenced this issue Feb 2, 2022
289: Fix regression where last release was not tested when last in search space r=foresterre a=foresterre

Fixes #288 

Co-authored-by: Martijn Gribnau <garm@ilumeo.com>
@bors bors bot closed this as completed in 494dd5c Feb 2, 2022
foresterre added a commit that referenced this issue Aug 1, 2022
289: Fix regression where last release was not tested when last in search space r=foresterre a=foresterre

Fixes #288 

Co-authored-by: Martijn Gribnau <garm@ilumeo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant