Skip to content

Commit

Permalink
Use inline snapshots, use assert_eq for requires-python tests, erro…
Browse files Browse the repository at this point in the history 
…r on requires python without lower bound
  • Loading branch information
@MichaReiser
MichaReiser committed Feb 12, 2025
1 parent 373d79f commit 8aae983
Show file tree
Hide file tree
Showing 15 changed files with 215 additions and 201 deletions.
232 changes: 199 additions & 33 deletions crates/red_knot_project/src/metadata.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::sync::Arc;
use thiserror::Error;

use crate::combine::Combine;
use crate::metadata::pyproject::{Project, PyProject, PyProjectError, TooLargeRequiresPythonError};
use crate::metadata::pyproject::{Project, PyProject, PyProjectError, ResolveRequiresPythonError};
use crate::metadata::value::ValueSource;
use options::KnotTomlError;
use options::Options;
Expand Down Expand Up @@ -52,7 +52,7 @@ impl ProjectMetadata {
pub(crate) fn from_pyproject(
pyproject: PyProject,
root: SystemPathBuf,
) -> Result<Self, TooLargeRequiresPythonError> {
) -> Result<Self, ResolveRequiresPythonError> {
Self::from_options(
pyproject
.tool
Expand All @@ -68,13 +68,14 @@ impl ProjectMetadata {
mut options: Options,
root: SystemPathBuf,
project: Option<&Project>,
) -> Result<Self, TooLargeRequiresPythonError> {
) -> Result<Self, ResolveRequiresPythonError> {
let name = project
.and_then(|project| project.name.as_deref())
.map(|name| Name::new(&**name))
.unwrap_or_else(|| Name::new(root.file_name().unwrap_or("root")));

// If the project doesn't specify a python version but the `project.requires-python` field is set, resolve the version
// If the `options` don't specify a python version but the `project.requires-python` field is set,
// use that as a lower bound instead.
if let Some(project) = project {
if !options
.environment
Expand Down Expand Up @@ -295,9 +296,9 @@ pub enum ProjectDiscoveryError {
path: SystemPathBuf,
},

#[error("the `requires-python` constraint in `{path}` is invalid: {source}")]
#[error("Invalid `requires-python` version specifier (`{path}`): {source}")]
InvalidRequiresPythonConstraint {
source: TooLargeRequiresPythonError,
source: ResolveRequiresPythonError,
path: SystemPathBuf,
},
}
Expand All @@ -306,9 +307,9 @@ pub enum ProjectDiscoveryError {
mod tests {
//! Integration tests for project discovery
use crate::snapshot_project;
use anyhow::{anyhow, Context};
use insta::assert_ron_snapshot;
use red_knot_python_semantic::PythonVersion;
use ruff_db::system::{SystemPathBuf, TestSystem};

use crate::{ProjectDiscoveryError, ProjectMetadata};
Expand All @@ -328,7 +329,15 @@ mod tests {

assert_eq!(project.root(), &*root);

snapshot_project!(project);
with_escaped_paths(|| {
assert_ron_snapshot!(&project, @r#"
ProjectMetadata(
name: Name("app"),
root: "/app",
options: Options(),
)
"#);
});

Ok(())
}
Expand Down Expand Up @@ -357,7 +366,16 @@ mod tests {
ProjectMetadata::discover(&root, &system).context("Failed to discover project")?;

assert_eq!(project.root(), &*root);
snapshot_project!(project);

with_escaped_paths(|| {
assert_ron_snapshot!(&project, @r#"
ProjectMetadata(
name: Name("backend"),
root: "/app",
options: Options(),
)
"#);
});

// Discovering the same package from a subdirectory should give the same result
let from_src = ProjectMetadata::discover(&root.join("db"), &system)
Expand Down Expand Up @@ -440,7 +458,19 @@ expected `.`, `]`

let sub_project = ProjectMetadata::discover(&root.join("packages/a"), &system)?;

snapshot_project!(sub_project);
with_escaped_paths(|| {
assert_ron_snapshot!(sub_project, @r#"
ProjectMetadata(
name: Name("nested-project"),
root: "/app/packages/a",
options: Options(
src: Some(SrcOptions(
root: Some("src"),
)),
),
)
"#);
});

Ok(())
}
Expand Down Expand Up @@ -478,7 +508,19 @@ expected `.`, `]`

let root = ProjectMetadata::discover(&root, &system)?;

snapshot_project!(root);
with_escaped_paths(|| {
assert_ron_snapshot!(root, @r#"
ProjectMetadata(
name: Name("project-root"),
root: "/app",
options: Options(
src: Some(SrcOptions(
root: Some("src"),
)),
),
)
"#);
});

Ok(())
}
Expand Down Expand Up @@ -510,7 +552,15 @@ expected `.`, `]`

let sub_project = ProjectMetadata::discover(&root.join("packages/a"), &system)?;

snapshot_project!(sub_project);
with_escaped_paths(|| {
assert_ron_snapshot!(sub_project, @r#"
ProjectMetadata(
name: Name("nested-project"),
root: "/app/packages/a",
options: Options(),
)
"#);
});

Ok(())
}
Expand Down Expand Up @@ -545,7 +595,19 @@ expected `.`, `]`

let root = ProjectMetadata::discover(&root.join("packages/a"), &system)?;

snapshot_project!(root);
with_escaped_paths(|| {
assert_ron_snapshot!(root, @r#"
ProjectMetadata(
name: Name("project-root"),
root: "/app",
options: Options(
environment: Some(EnvironmentOptions(
r#python-version: Some("3.10"),
)),
),
)
"#);
});

Ok(())
}
Expand Down Expand Up @@ -585,12 +647,27 @@ expected `.`, `]`

let root = ProjectMetadata::discover(&root, &system)?;

snapshot_project!(root);
with_escaped_paths(|| {
assert_ron_snapshot!(root, @r#"
ProjectMetadata(
name: Name("super-app"),
root: "/app",
options: Options(
environment: Some(EnvironmentOptions(
r#python-version: Some("3.12"),
)),
src: Some(SrcOptions(
root: Some("src"),
)),
),
)
"#);
});

Ok(())
}
#[test]
fn requires_python_no_python_version() -> anyhow::Result<()> {
fn requires_python_major_minor() -> anyhow::Result<()> {
let system = TestSystem::default();
let root = SystemPathBuf::from("/app");

Expand All @@ -607,7 +684,14 @@ expected `.`, `]`

let root = ProjectMetadata::discover(&root, &system)?;

snapshot_project!(root);
assert_eq!(
root.options
.environment
.unwrap_or_default()
.python_version
.as_deref(),
Some(&PythonVersion::PY312)
);

Ok(())
}
Expand All @@ -630,7 +714,14 @@ expected `.`, `]`

let root = ProjectMetadata::discover(&root, &system)?;

snapshot_project!(root);
assert_eq!(
root.options
.environment
.unwrap_or_default()
.python_version
.as_deref(),
Some(&PythonVersion::from((3, 0)))
);

Ok(())
}
Expand All @@ -655,7 +746,14 @@ expected `.`, `]`

let root = ProjectMetadata::discover(&root, &system)?;

snapshot_project!(root);
assert_eq!(
root.options
.environment
.unwrap_or_default()
.python_version
.as_deref(),
Some(&PythonVersion::PY312)
);

Ok(())
}
Expand All @@ -678,13 +776,20 @@ expected `.`, `]`

let root = ProjectMetadata::discover(&root, &system)?;

snapshot_project!(root);
assert_eq!(
root.options
.environment
.unwrap_or_default()
.python_version
.as_deref(),
Some(&PythonVersion::PY313)
);

Ok(())
}

#[test]
fn requires_greater_than_major_minor() -> anyhow::Result<()> {
fn requires_python_greater_than_major_minor() -> anyhow::Result<()> {
let system = TestSystem::default();
let root = SystemPathBuf::from("/app");

Expand All @@ -703,7 +808,14 @@ expected `.`, `]`

let root = ProjectMetadata::discover(&root, &system)?;

snapshot_project!(root);
assert_eq!(
root.options
.environment
.unwrap_or_default()
.python_version
.as_deref(),
Some(&PythonVersion::PY312)
);

Ok(())
}
Expand All @@ -730,7 +842,64 @@ expected `.`, `]`

let root = ProjectMetadata::discover(&root, &system)?;

snapshot_project!(root);
assert_eq!(
root.options
.environment
.unwrap_or_default()
.python_version
.as_deref(),
Some(&PythonVersion::PY310)
);

Ok(())
}

#[test]
fn requires_python_less_than() -> anyhow::Result<()> {
let system = TestSystem::default();
let root = SystemPathBuf::from("/app");

system
.memory_file_system()
.write_file(
root.join("pyproject.toml"),
r#"
[project]
requires-python = "<3.12"
"#,
)
.context("Failed to write file")?;

let Err(error) = ProjectMetadata::discover(&root, &system) else {
return Err(anyhow!("Expected project discovery to fail because the `requires-python` doesn't specify a lower bound (it only specifies an upper bound)."));
};

assert_error_eq(&error, "Invalid `requires-python` version specifier (`/app/pyproject.toml`): value `<3.12` does not contain a lower bound. Add a lower bound to indicate the minimum compatible Python version (e.g., `>=3.13`).");

Ok(())
}

#[test]
fn requires_python_no_specifiers() -> anyhow::Result<()> {
let system = TestSystem::default();
let root = SystemPathBuf::from("/app");

system
.memory_file_system()
.write_file(
root.join("pyproject.toml"),
r#"
[project]
requires-python = ""
"#,
)
.context("Failed to write file")?;

let Err(error) = ProjectMetadata::discover(&root, &system) else {
return Err(anyhow!("Expected project discovery to fail because the `requires-python` specifiers are empty and don't define a lower bound."));
};

assert_error_eq(&error, "Invalid `requires-python` version specifier (`/app/pyproject.toml`): value `` does not contain a lower bound. Add a lower bound to indicate the minimum compatible Python version (e.g., `>=3.13`).");

Ok(())
}
Expand All @@ -755,7 +924,7 @@ expected `.`, `]`
return Err(anyhow!("Expected project discovery to fail because of the requires-python major version that is larger than 255."));
};

assert_error_eq(&error, "the `requires-python` constraint in `/app/pyproject.toml` is invalid: The major version `999` is larger than the maximum supported value 255");
assert_error_eq(&error, "Invalid `requires-python` version specifier (`/app/pyproject.toml`): The major version `999` is larger than the maximum supported value 255");

Ok(())
}
Expand All @@ -765,15 +934,12 @@ expected `.`, `]`
assert_eq!(error.to_string().replace('\\', "/"), message);
}

/// Snapshots a project but with all paths using unix separators.
#[macro_export]
macro_rules! snapshot_project {
($project:expr) => {{
assert_ron_snapshot!($project,{
".root" => insta::dynamic_redaction(|content, _content_path| {
content.as_str().unwrap().replace("\\", "/")
}),
fn with_escaped_paths<R>(f: impl FnOnce() -> R) -> R {
let mut settings = insta::Settings::clone_current();
settings.add_dynamic_redaction(".root", |content, _path| {
content.as_str().unwrap().replace('\\', "/")
});
}};
}

settings.bind(f)
}
}
Loading

0 comments on commit 8aae983

Please sign in to comment.