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

Consolidate integration tests #1288

Merged
merged 5 commits into from
Oct 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,9 @@ jobs:
echo /Applications/Xcode* | sort | head -n1 | xargs rm -rf
# du -sh /*/* 2>/dev/null | sort -h || true

- name: Preinstall toolchains
run: cargo run -p dylint_internal --bin preinstall-toolchains

- name: Test
run: |
if [[ '${{ matrix.package }}' =~ ^cargo-dylint ]]; then
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use dylint_internal::{
use std::fs::create_dir_all;
use tempfile::tempdir_in;

#[cfg_attr(dylint_lib = "general", allow(non_thread_safe_call_in_test))]
#[test]
fn dylint_driver_path() {
let tempdir = tempdir_in(env!("CARGO_MANIFEST_DIR")).unwrap();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ fn main() {
}
"#;

#[cfg_attr(dylint_lib = "general", allow(non_thread_safe_call_in_test))]
#[test]
fn fix() {
let tempdir = tempdir().unwrap();
Expand Down
File renamed without changes.
9 changes: 9 additions & 0 deletions cargo-dylint/tests/integration/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
mod depinfo_dylint_libs;
mod dylint_driver_path;
mod fix;
mod library_packages;
mod list;
mod nightly_toolchain;
mod no_deps;
mod package_options;
mod warn;
File renamed without changes.
46 changes: 46 additions & 0 deletions driver/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

67 changes: 55 additions & 12 deletions dylint/src/driver_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ use dylint_internal::{
use semver::Version;
use std::{
env::consts,
fs::{copy, create_dir_all, write},
fs::{copy, create_dir_all, rename, write},
path::{Path, PathBuf},
};
use tempfile::tempdir;
use tempfile::{tempdir, NamedTempFile};

include!(concat!(env!("OUT_DIR"), "/dylint_driver_manifest_dir.rs"));

Expand Down Expand Up @@ -87,7 +87,7 @@ pub fn get(opts: &opts::Dylint, toolchain: &str) -> Result<PathBuf> {

let driver = driver_dir.join("dylint-driver");
if !driver.exists() || is_outdated(opts, toolchain, &driver)? {
build(opts, toolchain, &driver)?;
build(opts, toolchain, &driver_dir)?;
}

Ok(driver)
Expand Down Expand Up @@ -142,7 +142,7 @@ fn is_outdated(opts: &opts::Dylint, toolchain: &str, driver: &Path) -> Result<bo
}

#[cfg_attr(dylint_lib = "supplementary", allow(commented_code))]
fn build(opts: &opts::Dylint, toolchain: &str, driver: &Path) -> Result<()> {
fn build(opts: &opts::Dylint, toolchain: &str, driver_dir: &Path) -> Result<()> {
let tempdir = tempdir().with_context(|| "`tempdir` failed")?;
let package = tempdir.path();

Expand Down Expand Up @@ -182,14 +182,30 @@ fn build(opts: &opts::Dylint, toolchain: &str, driver: &Path) -> Result<()> {
.target_directory
.join("debug")
.join(format!("dylint_driver-{toolchain}{}", consts::EXE_SUFFIX));

let named_temp_file = NamedTempFile::new_in(driver_dir)
.with_context(|| "Could not create temporary directory")?;

#[cfg_attr(dylint_lib = "general", allow(non_thread_safe_call_in_test))]
copy(&binary, driver).with_context(|| {
copy(&binary, &named_temp_file).with_context(|| {
format!(
"Could not copy `{binary}` to `{}`",
driver.to_string_lossy()
named_temp_file.path().to_string_lossy()
)
})?;

let driver = driver_dir.join("dylint-driver");

// smoelius: Windows requires that the old file be moved out of the way first.
if cfg!(target_os = "windows") {
let temp_path = NamedTempFile::new_in(driver_dir)
.map(NamedTempFile::into_temp_path)
.with_context(|| "Could not create temporary directory")?;
rename(&driver, &temp_path).unwrap_or_default();
}

named_temp_file.persist(&driver)?;

Ok(())
}

Expand Down Expand Up @@ -228,17 +244,44 @@ fn initialize(toolchain: &str, package: &Path) -> Result<()> {
#[cfg(test)]
mod test {
use super::*;
use std::process::{Command, ExitStatus};

// smoelius: `tempdir` is a temporary directory. So there should be no race here.
#[cfg_attr(dylint_lib = "general", allow(non_thread_safe_call_in_test))]
#[test]
fn nightly() {
let tempdir = tempdir().unwrap();
build(
&opts::Dylint::default(),
"nightly",
&tempdir.path().join("dylint-driver"),
)
.unwrap();
build(&opts::Dylint::default(), "nightly", tempdir.path()).unwrap();
}

// smoelius: As mentioned above, `tempdir` is a temporary directory. So there should be no race
// here.
#[cfg_attr(dylint_lib = "general", allow(non_thread_safe_call_in_test))]
#[test]
fn can_install_while_driver_is_running() {
const WHICH: &str = if cfg!(target_os = "windows") {
"where"
} else {
"which"
};

let tempdir = tempdir().unwrap();
let driver = tempdir.path().join("dylint-driver");

// Set tmpdir/dylint-driver to `sleep` and call it with `infinity`.
let stdout = Command::new(WHICH)
.arg("sleep")
.logged_output(false)
.unwrap()
.stdout;
let sleep_path = String::from_utf8(stdout).unwrap();
copy(sleep_path.trim_end(), &driver).unwrap();
let mut child = Command::new(driver).arg("infinity").spawn().unwrap();

// Install should not fail with "text file busy".
build(&opts::Dylint::default(), "nightly", tempdir.path()).unwrap();

child.kill().unwrap();
let _: ExitStatus = child.wait().unwrap();
}
}
52 changes: 48 additions & 4 deletions examples/general/crate_wide_allow/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,28 @@ mod test {
use cargo_metadata::MetadataCommand;
use dylint_internal::env;
use predicates::prelude::*;
use std::{env::consts, sync::Mutex};
use std::{
env::consts,
sync::{Mutex, MutexGuard},
};

static MUTEX: Mutex<()> = Mutex::new(());
fn mutex<T: maybe_return::MaybeReturn<MutexGuard<'static, ()>>>() -> T::Output {
static MUTEX: Mutex<()> = Mutex::new(());

let lock = MUTEX.lock().unwrap();

// smoelius: Ensure the `clippy` component is installed.
Command::new("rustup")
.args(["component", "add", "clippy"])
.assert()
.success();

T::maybe_return(lock)
}

#[test]
fn ui() {
let _lock = MUTEX.lock().unwrap();
let _lock = mutex::<maybe_return::Yes>();

dylint_testing::ui_test_example(env!("CARGO_PKG_NAME"), "ui");
}
Expand Down Expand Up @@ -115,7 +130,7 @@ mod test {
fn test(rustflags: &str, assert: impl Fn(Assert) -> Assert) {
const MANIFEST_DIR: &str = "../../..";

let _lock = MUTEX.lock().unwrap();
let _lock = mutex::<maybe_return::Yes>();

Command::new("cargo")
.current_dir(MANIFEST_DIR)
Expand Down Expand Up @@ -159,6 +174,8 @@ mod test {

#[test]
fn premise_manifest_sanity() {
mutex::<maybe_return::No>();

let mut command = Command::new("cargo");
command.args(["clippy"]);
command.current_dir("ui_manifest");
Expand All @@ -171,6 +188,8 @@ mod test {
/// Verify that `allow`ing a lint in the manifest does not silently override `--warn`.
#[test]
fn premise_manifest_warn() {
mutex::<maybe_return::No>();

let mut command = Command::new("cargo");
command.args(["clippy", "--", "--warn=clippy::assertions-on-constants"]);
command.current_dir("ui_manifest");
Expand All @@ -183,6 +202,8 @@ mod test {
/// Verify that `allow`ing a lint in the manifest does not silently override `--deny`.
#[test]
fn premise_manifest_deny() {
mutex::<maybe_return::No>();

let mut command = Command::new("cargo");
command.args(["clippy", "--", "--deny=clippy::assertions-on-constants"]);
command.current_dir("ui_manifest");
Expand All @@ -191,4 +212,27 @@ mod test {
.failure()
.stderr(predicate::str::contains(ASSERTIONS_ON_CONSTANTS_WARNING));
}

mod maybe_return {
pub trait MaybeReturn<T> {
type Output;
fn maybe_return(value: T) -> Self::Output;
}

pub struct Yes;

pub struct No;

impl<T> MaybeReturn<T> for Yes {
type Output = T;
fn maybe_return(value: T) -> Self::Output {
value
}
}

impl<T> MaybeReturn<T> for No {
type Output = ();
fn maybe_return(_value: T) -> Self::Output {}
}
}
}
1 change: 1 addition & 0 deletions examples/testing/marker/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 3 additions & 4 deletions internal/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ repository = "https://github.com/trailofbits/dylint"

[dependencies]
anyhow = "1.0"
once_cell = "1.19"
regex = "1.10"

ansi_term = { version = "0.12", optional = true }
bitflags = { version = "2.6", optional = true }
Expand All @@ -21,8 +23,6 @@ home = { version = "0.5", optional = true }
if_chain = { version = "1.0", optional = true }
is-terminal = { version = "0.4", optional = true }
log = { version = "0.4", optional = true }
once_cell = { version = "1.19", optional = true }
regex = { version = "1.10", optional = true }
rust-embed = { version = "8.5", features = [
"include-exclude",
], optional = true }
Expand All @@ -44,7 +44,6 @@ cargo = [
"command",
"home",
"is-terminal",
"once_cell",
]
clippy_utils = ["semver", "toml_edit"]
command = ["log"]
Expand All @@ -53,7 +52,7 @@ examples = ["cargo", "cargo-util", "rustup", "walkdir"]
git = ["command", "git2", "if_chain"]
packaging = ["cargo", "rust-embed"]
rustup = ["command"]
sed = ["regex"]
sed = []
testing = ["ctor", "env_logger", "packaging"]

[lints]
Expand Down
Loading