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

rustfix: Fix ignored tests #13034

Closed
ehuss opened this issue Nov 22, 2023 · 0 comments · Fixed by #13047
Closed

rustfix: Fix ignored tests #13034

ehuss opened this issue Nov 22, 2023 · 0 comments · Fixed by #13047
Labels
A-testing-cargo-itself Area: cargo's tests Command-fix S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review

Comments

@ehuss
Copy link
Contributor

ehuss commented Nov 22, 2023

For some reason, several tests in rustfix are ignored:

  • edition tests: https://github.com/rust-lang/cargo/blob/65d0eb536dd4a7ae6ddc50ca14556896e450c4ff/crates/rustfix/tests/parse_and_replace.rs#L242C1-L247. There don't appear to be any edition tests, so I think all of that should just be removed.
  • These two proptests:
    proptest! {
    #[test]
    #[ignore]
    fn new_to_vec_roundtrip(ref s in "\\PC*") {
    assert_eq!(s.as_bytes(), Data::new(s.as_bytes()).to_vec().as_slice());
    }
    #[test]
    #[ignore]
    fn replace_random_chunks(
    ref data in "\\PC*",
    ref replacements in prop::collection::vec(
    (any::<::std::ops::Range<usize>>(), any::<Vec<u8>>()),
    1..1337,
    )
    ) {
    let mut d = Data::new(data.as_bytes());
    for &(ref range, ref bytes) in replacements {
    let _ = d.replace_range(range.clone(), bytes);
    }
    }
    }
    }
    . I'm guessing that these are ignored because they were either slow, or had errors. However, in my testing they seem to run pretty fast, so I'm guessing we could just remove the #[ignore]. This needs a little investigation, just to make sure these tests aren't flaky or anything.
@ehuss ehuss added A-testing-cargo-itself Area: cargo's tests S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review Command-fix labels Nov 22, 2023
@weihanglo weihanglo linked a pull request Nov 26, 2023 that will close this issue
bors added a commit that referenced this issue Nov 26, 2023
review and remove ignored tests in rustfix

### What does this PR try to resolve?
review ignored tests in rustfix crate per #13034.

### How should we test and review this PR?
CI testing

### Additional information

* Removed unproductive test in `parse_and_replace`
* un-ignore proptests, and reduce runtime from ~2s to ~<.25s
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testing-cargo-itself Area: cargo's tests Command-fix S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant