From 2187a905db73d0a488e3650b7620386f20a87f3d Mon Sep 17 00:00:00 2001 From: Ed Morley <501702+edmorley@users.noreply.github.com> Date: Fri, 26 Jul 2024 14:51:25 +0100 Subject: [PATCH] Improve tests for buildpack env var handling * 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. --- src/layers/pip_dependencies.rs | 6 ++-- src/layers/python.rs | 55 +++++----------------------------- tests/mod.rs | 20 +++++++++++-- tests/pip_test.rs | 24 +++++++++++++-- tests/python_version_test.rs | 17 ++--------- 5 files changed, 52 insertions(+), 70 deletions(-) diff --git a/src/layers/pip_dependencies.rs b/src/layers/pip_dependencies.rs index 326fe08..519ca50 100644 --- a/src/layers/pip_dependencies.rs +++ b/src/layers/pip_dependencies.rs @@ -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")] ); } } diff --git a/src/layers/python.rs b/src/layers/python.rs index df578a4..3f8fe1e 100644 --- a/src/layers/python.rs +++ b/src/layers/python.rs @@ -562,43 +562,6 @@ 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"); @@ -606,10 +569,9 @@ mod tests { 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, @@ -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"), ] ); } diff --git a/tests/mod.rs b/tests/mod.rs index 7f93f55..d9f7ead 100644 --- a/tests/mod.rs +++ b/tests/mod.rs @@ -25,6 +25,7 @@ const DEFAULT_BUILDER: &str = "heroku/builder:24"; fn default_build_config(fixture_path: impl AsRef) -> 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). @@ -33,9 +34,24 @@ fn default_build_config(fixture_path: impl AsRef) -> 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 } diff --git a/tests/pip_test.rs b/tests/pip_test.rs index 2b350d7..4f74985 100644 --- a/tests/pip_test.rs +++ b/tests/pip_test.rs @@ -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} diff --git a/tests/python_version_test.rs b/tests/python_version_test.rs index c881af6..96e0b77 100644 --- a/tests/python_version_test.rs +++ b/tests/python_version_test.rs @@ -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, @@ -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