Skip to content

Commit

Permalink
Add scenario tests for pip-compile (#1011)
Browse files Browse the repository at this point in the history
e.g. for scenarios that test resolution _without_ installation.

This refactors the `update` script to generate scenario test files for
`pip compile` _and_ `pip install`. We don't overlap scenarios to save
time. We only generate `pip compile` test cases for scenarios we cannot
represent with `pip install` e.g. a `--python-version` override.

The _one_ scenario I added happened to reveal a bug in our resolver
where we were incorrectly filtering versions by the installed version
when wheels were available. Per the comment at
#883 (comment),
we should _only_ need to check for a compatible installed Python version
when using a different _target_ Python version if we need to build a
source distribution.
53bce68
resolves this by removing the excessive constraints — the correct Python
version incompatibilities are applied elsewhere.
  • Loading branch information
zanieb authored Jan 21, 2024
1 parent d9cc9db commit 4026710
Show file tree
Hide file tree
Showing 6 changed files with 226 additions and 56 deletions.
16 changes: 0 additions & 16 deletions crates/puffin-resolver/src/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -652,22 +652,6 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
self.visit_package(package, priorities, request_sink)?;
}

// If a package has a `requires_python` field, add a constraint on the target
// Python version.
if let Some(requires_python) = metadata.requires_python.as_ref() {
let version = requires_python
.iter()
.map(PubGrubSpecifier::try_from)
.fold_ok(Range::full(), |range, specifier| {
range.intersection(&specifier.into())
})?;
constraints.insert(
PubGrubPackage::Python(PubGrubPython::Installed),
version.clone(),
);
constraints.insert(PubGrubPackage::Python(PubGrubPython::Target), version);
}

// If a package has an extra, insert a constraint on the base package.
if extra.is_some() {
constraints.insert(
Expand Down
78 changes: 78 additions & 0 deletions crates/puffin/tests/pip_compile_scenarios.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
//! DO NOT EDIT
//!
//! Generated with ./scripts/scenarios/update.py
//! Scenarios from <https://github.com/zanieb/packse/tree/9a836122ad43eb9c8115ef09f3beb7779512cd78/scenarios>
//!
#![cfg(all(feature = "python", feature = "pypi"))]

use std::process::Command;

use anyhow::Result;
use assert_fs::fixture::{FileWriteStr, PathChild};
use insta_cmd::_macro_support::insta;
use insta_cmd::{assert_cmd_snapshot, get_cargo_bin};

use common::{create_venv, BIN_NAME, INSTA_FILTERS};

mod common;

/// requires-python-version-greater-than-current-resolver-override
///
/// The user requires a package which requires a Python version greater than the
/// current version, but they use an alternative Python version for package
/// resolution.
///
/// ```text
/// b6505624
/// ├── environment
/// │ └── python3.9
/// ├── root
/// │ └── requires a==1.0.0
/// │ └── satisfied by a-1.0.0
/// └── a
/// └── a-1.0.0
/// └── requires python>=3.10 (incompatible with environment)
/// ```
#[test]
fn requires_python_version_greater_than_current_resolver_override() -> Result<()> {
let temp_dir = assert_fs::TempDir::new()?;
let cache_dir = assert_fs::TempDir::new()?;
let venv = create_venv(&temp_dir, &cache_dir, "python3.9");

// In addition to the standard filters, swap out package names for more realistic messages
let mut filters = INSTA_FILTERS.to_vec();
filters.push((r"a-b6505624", "albatross"));
filters.push((r"-b6505624", ""));

let requirements_in = temp_dir.child("requirements.in");
requirements_in.write_str("a-b6505624==1.0.0")?;

insta::with_settings!({
filters => filters
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.arg("pip")
.arg("compile")
.arg("requirements.in")
.arg("--python-version=3.11")
.arg("--extra-index-url")
.arg("https://test.pypi.org/simple")
.arg("--cache-dir")
.arg(cache_dir.path())
.env("VIRTUAL_ENV", venv.as_os_str())
.env("PUFFIN_NO_WRAP", "1")
.current_dir(&temp_dir), @r###"
success: true
exit_code: 0
----- stdout -----
# This file was autogenerated by Puffin v[VERSION] via the following command:
# puffin pip compile requirements.in --python-version=3.11 --extra-index-url https://test.pypi.org/simple --cache-dir [CACHE_DIR]
albatross==1.0.0
----- stderr -----
Resolved 1 package in [TIME]
"###);
});

Ok(())
}
8 changes: 4 additions & 4 deletions crates/puffin/tests/pip_install_scenarios.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! DO NOT EDIT
//!
//! Generated with ./scripts/scenarios/update.py
//! Scenarios from <https://github.com/zanieb/packse/tree/b6cb1f6310a40937dc68a59c82460fea58957b70/scenarios>
//! Scenarios from <https://github.com/zanieb/packse/tree/9a836122ad43eb9c8115ef09f3beb7779512cd78/scenarios>
//!
#![cfg(all(feature = "python", feature = "pypi"))]

Expand Down Expand Up @@ -690,14 +690,14 @@ fn dependency_excludes_non_contiguous_range_of_compatible_versions() -> Result<(
crow>2.0.0
and crow==1.0.0 depends on albatross<2.0.0, we can conclude that crow<2.0.0 depends on albatross<2.0.0. (1)
Because albatross==1.0.0 depends on bluebird==1.0.0 and there are no versions of albatross that satisfy any of:
Because there are no versions of albatross that satisfy any of:
albatross<1.0.0
albatross>1.0.0,<2.0.0
we can conclude that albatross<2.0.0 depends on bluebird==1.0.0.
and albatross==1.0.0 depends on bluebird==1.0.0, we can conclude that albatross<2.0.0 depends on bluebird==1.0.0.
And because we know from (1) that crow<2.0.0 depends on albatross<2.0.0, we can conclude that crow<2.0.0 depends on bluebird==1.0.0.
And because crow==2.0.0 depends on albatross>=3.0.0 we can conclude that all versions of crow, bluebird!=1.0.0, albatross<3.0.0 are incompatible. (2)
Because albatross==3.0.0 depends on bluebird==3.0.0 and only albatross<=3.0.0 is available, we can conclude that albatross>=3.0.0 depends on bluebird==3.0.0.
Because only albatross<=3.0.0 is available and albatross==3.0.0 depends on bluebird==3.0.0, we can conclude that albatross>=3.0.0 depends on bluebird==3.0.0.
And because we know from (2) that all versions of crow, bluebird!=1.0.0, albatross<3.0.0 are incompatible, we can conclude that all versions of crow depend on one of:
bluebird<=1.0.0
bluebird>=3.0.0
Expand Down
78 changes: 78 additions & 0 deletions scripts/scenarios/templates/compile.mustache
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
//! DO NOT EDIT
//!
//! Generated with {{generated_with}}
//! Scenarios from <{{generated_from}}>
//!
#![cfg(all(feature = "python", feature = "pypi"))]

use std::process::Command;

use anyhow::Result;
use assert_fs::fixture::{FileWriteStr, PathChild};
use insta_cmd::_macro_support::insta;
use insta_cmd::{assert_cmd_snapshot, get_cargo_bin};

use common::{create_venv, BIN_NAME, INSTA_FILTERS};

mod common;
{{#scenarios}}

/// {{name}}
///
{{#description_lines}}
/// {{.}}
{{/description_lines}}
///
/// ```text
/// {{version}}
{{#tree}}
/// {{.}}
{{/tree}}
/// ```
#[test]
fn {{module_name}}() -> Result<()> {
let temp_dir = assert_fs::TempDir::new()?;
let cache_dir = assert_fs::TempDir::new()?;
let venv = create_venv(&temp_dir, &cache_dir, "python{{environment.python}}");
// In addition to the standard filters, swap out package names for more realistic messages
let mut filters = INSTA_FILTERS.to_vec();
{{#packages}}
filters.push((r"{{name}}", "{{cute_name}}"));
{{/packages}}
filters.push((r"-{{version}}", ""));

let requirements_in = temp_dir.child("requirements.in");
{{#root.requires}}
requirements_in.write_str("{{requirement}}")?;
{{/root.requires}}

{{#expected.explanation_lines}}
// {{.}}
{{/expected.explanation_lines}}
insta::with_settings!({
filters => filters
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.arg("pip")
.arg("compile")
.arg("requirements.in")
{{#environment.prereleases}}
.arg("--prerelease=allow")
{{/environment.prereleases}}
{{#resolver_options.python}}
.arg("--python-version={{.}}")
{{/resolver_options.python}}
.arg("--extra-index-url")
.arg("https://test.pypi.org/simple")
.arg("--cache-dir")
.arg(cache_dir.path())
.env("VIRTUAL_ENV", venv.as_os_str())
.env("PUFFIN_NO_WRAP", "1")
.current_dir(&temp_dir), @r###"<snapshot>
"###);
});

Ok(())
}
{{/scenarios}}
File renamed without changes.
102 changes: 66 additions & 36 deletions scripts/scenarios/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,25 @@

import json
import shutil
import os
import subprocess
import sys
import textwrap
from pathlib import Path


PACKSE_COMMIT = "b6cb1f6310a40937dc68a59c82460fea58957b70"
PACKSE_COMMIT = "9a836122ad43eb9c8115ef09f3beb7779512cd78"
TOOL_ROOT = Path(__file__).parent
TEMPLATE = TOOL_ROOT / "template.mustache"
TEMPLATES = TOOL_ROOT / "templates"
INSTALL_TEMPLATE = TEMPLATES / "install.mustache"
COMPILE_TEMPLATE = TEMPLATES / "compile.mustache"
PACKSE = TOOL_ROOT / "packse-scenarios"
REQUIREMENTS = TOOL_ROOT / "requirements.txt"
PROJECT_ROOT = TOOL_ROOT.parent.parent
TARGET = PROJECT_ROOT / "crates" / "puffin" / "tests" / "pip_install_scenarios.rs"
TESTS = PROJECT_ROOT / "crates" / "puffin" / "tests"
INSTALL_TESTS = TESTS / "pip_install_scenarios.rs"
COMPILE_TESTS = TESTS / "pip_compile_scenarios.rs"

CUTE_NAMES = {
"a": "albatross",
"b": "bluebird",
Expand Down Expand Up @@ -151,10 +157,6 @@
)
)

# Add generated metadata
data["generated_from"] = f"https://github.com/zanieb/packse/tree/{commit}/scenarios"
data["generated_with"] = " ".join(sys.argv)


# Drop the example scenario
for index, scenario in enumerate(data["scenarios"]):
Expand All @@ -181,35 +183,63 @@
package["cute_name"] = CUTE_NAMES[package["name"][0]]


# Render the template
print("Rendering template...", file=sys.stderr)
output = chevron_blue.render(
template=TEMPLATE.read_text(), data=data, no_escape=True, warn=True
)
# Split scenarios into `install` and `compile` cases
install_scenarios = []
compile_scenarios = []

# Update the test file
print(f"Updating test file at `{TARGET.relative_to(PROJECT_ROOT)}`...", file=sys.stderr)
with open(TARGET, "wt") as target_file:
target_file.write(output)

# Format
print("Formatting test file...", file=sys.stderr)
subprocess.check_call(["rustfmt", str(TARGET)])

# Update snapshots
print("Updating snapshots...\n", file=sys.stderr)
subprocess.call(
[
"cargo",
"insta",
"test",
"--accept",
"--test",
TARGET.with_suffix("").name,
"--features",
"pypi,python",
],
cwd=PROJECT_ROOT,
)
for scenario in data["scenarios"]:
if (scenario["resolver_options"] or {}).get("python") is not None:
compile_scenarios.append(scenario)
else:
install_scenarios.append(scenario)

for template, tests, scenarios in [
(INSTALL_TEMPLATE, INSTALL_TESTS, install_scenarios),
(COMPILE_TEMPLATE, COMPILE_TESTS, compile_scenarios),
]:
data = {"scenarios": scenarios}

# Add generated metadata
data["generated_from"] = f"https://github.com/zanieb/packse/tree/{commit}/scenarios"
data["generated_with"] = " ".join(sys.argv)

# Render the template
print(f"Rendering template {template.name}", file=sys.stderr)
output = chevron_blue.render(
template=template.read_text(), data=data, no_escape=True, warn=True
)

# Update the test files
print(
f"Updating test file at `{tests.relative_to(PROJECT_ROOT)}`...",
file=sys.stderr,
)
with open(tests, "wt") as test_file:
test_file.write(output)

# Format
print(
"Formatting test file...",
file=sys.stderr,
)
subprocess.check_call(["rustfmt", str(tests)])

# Update snapshots
print("Updating snapshots...\n", file=sys.stderr)
subprocess.call(
[
"cargo",
"insta",
"test",
"--features",
"pypi,python",
"--accept",
"--test-runner",
"nextest",
"--test",
tests.with_suffix("").name,
],
cwd=PROJECT_ROOT,
)

print("\nDone!", file=sys.stderr)

0 comments on commit 4026710

Please sign in to comment.