Skip to content

Commit

Permalink
Improve tests for buildpack env var handling
Browse files Browse the repository at this point in the history
* Shifts the testing of buildpack env vars handling slightly more
  towards integration tests rather than unit tests, since a big part of
  env var handling is ensuring the env is passed around correctly +
  aspects like the automatic env vars added by libcnb and lifecycle,
  which can only be properly tested via integration tests.
* Adds more testing of broken user-provided env vars, by passing
  them in by default for all tests (instead of a single test),
  and by adding more broken env vars to the list.
* In the unit tests, switches the example layer location to one that's
  more obviously an example path, to prevent anyone reading the
  tests from thinking that's the actual location set by the buildpack.

This change will help validate the env var handling remains consistent
with the upcoming transition to libcnb.rs' struct layer API.

GUS-W-16261336.
  • Loading branch information
edmorley committed Jul 26, 2024
1 parent 2add4a0 commit 2187a90
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 70 deletions.
6 changes: 3 additions & 3 deletions src/layers/pip_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,15 +151,15 @@ mod tests {
let mut base_env = Env::new();
base_env.insert("PYTHONUSERBASE", "this-should-be-overridden");

let layer_env = generate_layer_env(Path::new("/layers/dependencies"));
let layer_env = generate_layer_env(Path::new("/layer-dir"));

assert_eq!(
utils::environment_as_sorted_vector(&layer_env.apply(Scope::Build, &base_env)),
[("PYTHONUSERBASE", "/layers/dependencies")]
[("PYTHONUSERBASE", "/layer-dir")]
);
assert_eq!(
utils::environment_as_sorted_vector(&layer_env.apply(Scope::Launch, &base_env)),
[("PYTHONUSERBASE", "/layers/dependencies")]
[("PYTHONUSERBASE", "/layer-dir")]
);
}
}
55 changes: 8 additions & 47 deletions src/layers/python.rs
Original file line number Diff line number Diff line change
Expand Up @@ -562,54 +562,16 @@ mod tests {

#[test]
fn python_layer_env() {
let layer_env = generate_layer_env(
Path::new("/layers/python"),
&PythonVersion {
major: 3,
minor: 9,
patch: 0,
},
);

// Remember to force invalidation of the cached layer if these env vars ever change.
assert_eq!(
utils::environment_as_sorted_vector(&layer_env.apply_to_empty(Scope::Build)),
[
("CPATH", "/layers/python/include/python3.9"),
("LANG", "C.UTF-8"),
("PIP_DISABLE_PIP_VERSION_CHECK", "1"),
("PKG_CONFIG_PATH", "/layers/python/lib/pkgconfig"),
("PYTHONHOME", "/layers/python"),
("PYTHONUNBUFFERED", "1"),
("SOURCE_DATE_EPOCH", "315532801"),
]
);
assert_eq!(
utils::environment_as_sorted_vector(&layer_env.apply_to_empty(Scope::Launch)),
[
("CPATH", "/layers/python/include/python3.9"),
("LANG", "C.UTF-8"),
("PIP_DISABLE_PIP_VERSION_CHECK", "1"),
("PKG_CONFIG_PATH", "/layers/python/lib/pkgconfig"),
("PYTHONHOME", "/layers/python"),
("PYTHONUNBUFFERED", "1"),
]
);
}

#[test]
fn python_layer_env_with_existing_env() {
let mut base_env = Env::new();
base_env.insert("CPATH", "/base");
base_env.insert("LANG", "this-should-be-overridden");
base_env.insert("PIP_DISABLE_PIP_VERSION_CHECK", "this-should-be-overridden");
base_env.insert("PKG_CONFIG_PATH", "/base");
base_env.insert("PYTHONHOME", "this-should-be-overridden");
base_env.insert("PYTHONUNBUFFERED", "this-should-be-overridden");
base_env.insert("SOURCE_DATE_EPOCH", "this-should-be-preserved");

let layer_env = generate_layer_env(
Path::new("/layers/python"),
Path::new("/layer-dir"),
&PythonVersion {
major: 3,
minor: 11,
Expand All @@ -621,25 +583,24 @@ mod tests {
assert_eq!(
utils::environment_as_sorted_vector(&layer_env.apply(Scope::Build, &base_env)),
[
("CPATH", "/layers/python/include/python3.11:/base"),
("CPATH", "/layer-dir/include/python3.11:/base"),
("LANG", "C.UTF-8"),
("PIP_DISABLE_PIP_VERSION_CHECK", "1"),
("PKG_CONFIG_PATH", "/layers/python/lib/pkgconfig:/base"),
("PYTHONHOME", "/layers/python"),
("PKG_CONFIG_PATH", "/layer-dir/lib/pkgconfig:/base"),
("PYTHONHOME", "/layer-dir"),
("PYTHONUNBUFFERED", "1"),
("SOURCE_DATE_EPOCH", "this-should-be-preserved"),
("SOURCE_DATE_EPOCH", "315532801"),
]
);
assert_eq!(
utils::environment_as_sorted_vector(&layer_env.apply(Scope::Launch, &base_env)),
[
("CPATH", "/layers/python/include/python3.11:/base"),
("CPATH", "/layer-dir/include/python3.11:/base"),
("LANG", "C.UTF-8"),
("PIP_DISABLE_PIP_VERSION_CHECK", "1"),
("PKG_CONFIG_PATH", "/layers/python/lib/pkgconfig:/base"),
("PYTHONHOME", "/layers/python"),
("PKG_CONFIG_PATH", "/layer-dir/lib/pkgconfig:/base"),
("PYTHONHOME", "/layer-dir"),
("PYTHONUNBUFFERED", "1"),
("SOURCE_DATE_EPOCH", "this-should-be-preserved"),
]
);
}
Expand Down
20 changes: 18 additions & 2 deletions tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const DEFAULT_BUILDER: &str = "heroku/builder:24";

fn default_build_config(fixture_path: impl AsRef<Path>) -> BuildConfig {
let builder = builder();
let mut config = BuildConfig::new(&builder, fixture_path);

// TODO: Once Pack build supports `--platform` and libcnb-test adjusted accordingly, change this
// to allow configuring the target arch independently of the builder name (eg via env var).
Expand All @@ -33,9 +34,24 @@ fn default_build_config(fixture_path: impl AsRef<Path>) -> BuildConfig {
"heroku/builder:24" if cfg!(target_arch = "aarch64") => "aarch64-unknown-linux-musl",
_ => "x86_64-unknown-linux-musl",
};

let mut config = BuildConfig::new(&builder, fixture_path);
config.target_triple(target_triple);

// Ensure that potentially broken user-provided env vars don't take precedence over those set
// by this buildpack and break running Python/Pip. Some of these are based on the env vars that
// used to be set by `bin/release` by very old versions of the classic Python buildpack:
// https://github.com/heroku/heroku-buildpack-python/blob/27abdfe7d7ad104dabceb45641415251e965671c/bin/release#L11-L18
config.envs([
("CPATH", "/invalid"),
("LD_LIBRARY_PATH", "/invalid"),
("LIBRARY_PATH", "/invalid"),
("PATH", "/invalid"),
("PIP_DISABLE_PIP_VERSION_CHECK", "0"),
("PKG_CONFIG_PATH", "/invalid"),
("PYTHONHOME", "/invalid"),
("PYTHONPATH", "/invalid"),
("PYTHONUSERBASE", "/invalid"),
]);

config
}

Expand Down
24 changes: 21 additions & 3 deletions tests/pip_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,19 +38,37 @@ fn pip_basic_install_and_cache_reuse() {
);

// Check that:
// - Pip is available at runtime too (and not just during the build).
// - The correct env vars are set at run-time.
// - Pip is available at run-time too (and not just during the build).
// - The correct versions of pip/setuptools/wheel were installed.
// - Pip uses (via 'PYTHONUSERBASE') the user site-packages in the dependencies
// layer, and so can find the typing-extensions package installed there.
// - The "pip update available" warning is not shown (since it should be suppressed).
// - The system site-packages directory is protected against running 'pip install'
// without having passed '--user'.
let command_output =
context.run_shell_command("pip list && pip install --dry-run typing-extensions");
let command_output = context.run_shell_command(
indoc! {"
set -euo pipefail
printenv | sort | grep -vE '^(_|HOME|HOSTNAME|OLDPWD|PWD|SHLVL)='
echo
pip list
pip install --dry-run typing-extensions
"}
);
assert_empty!(command_output.stderr);
assert_contains!(
command_output.stdout,
&formatdoc! {"
CPATH=/layers/heroku_python/python/include/python3.12
LANG=C.UTF-8
LD_LIBRARY_PATH=/layers/heroku_python/python/lib:/layers/heroku_python/dependencies/lib
PATH=/layers/heroku_python/python/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PIP_DISABLE_PIP_VERSION_CHECK=1
PKG_CONFIG_PATH=/layers/heroku_python/python/lib/pkgconfig
PYTHONHOME=/layers/heroku_python/python
PYTHONUNBUFFERED=1
PYTHONUSERBASE=/layers/heroku_python/dependencies
Package Version
----------------- -------
pip {pip_version}
Expand Down
17 changes: 2 additions & 15 deletions tests/python_version_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,20 +84,7 @@ fn builds_with_python_version(fixture_path: &str, python_version: &str) {
wheel_version,
} = PackagingToolVersions::default();

let mut config = default_build_config(fixture_path);
// Checks that potentially broken user-provided env vars don't take precedence over those
// set by this buildpack and break running Python. These are based on the env vars that
// used to be set by `bin/release` by very old versions of the classic Python buildpack:
// https://github.com/heroku/heroku-buildpack-python/blob/27abdfe7d7ad104dabceb45641415251e965671c/bin/release#L11-L18
config.envs([
("LD_LIBRARY_PATH", "/invalid-path"),
("LIBRARY_PATH", "/invalid-path"),
("PATH", "/invalid-path"),
("PYTHONHOME", "/invalid-path"),
("PYTHONPATH", "/invalid-path"),
]);

TestRunner::default().build(config, |context| {
TestRunner::default().build(default_build_config(fixture_path), |context| {
assert_empty!(context.pack_stderr);
assert_contains!(
context.pack_stdout,
Expand All @@ -113,7 +100,7 @@ fn builds_with_python_version(fixture_path: &str, python_version: &str) {
// There's no sensible default process type we can set for Python apps.
assert_contains!(context.pack_stdout, "no default process type");

// Validate that the Python install works as expected at runtime.
// Validate that the Python install works as expected at run-time.
let command_output = context.run_shell_command(
indoc! {r#"
set -euo pipefail
Expand Down

0 comments on commit 2187a90

Please sign in to comment.