Skip to content

Commit

Permalink
Only copy app_dir when a preprocessor is configured
Browse files Browse the repository at this point in the history
  • Loading branch information
Malax committed Jun 24, 2022
1 parent 9e7cee8 commit debff59
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 19 deletions.
3 changes: 2 additions & 1 deletion libcnb-test/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
- Add `TestContext::run_test`, allowing you to run subsequent integration tests with the image from a previous test. These functions allow testing of subsequent builds, including caching logic and buildpack behaviour when build environment variables change, stacks are upgraded and more. ([#422](https://github.com/heroku/libcnb.rs/pull/422)).
- Add `TestConfig::expected_pack_result`. When set to `PackResult::Failure`, it allows testing of build failure scenarios. ([#429](https://github.com/heroku/libcnb.rs/pull/429)).
- Add `TestConfig::app_dir` which is handy in cases where `TestConfig` values are shared and only the `app_dir` needs to be different. ([#430](https://github.com/heroku/libcnb.rs/pull/430)).
- Remove `TestContext::app_dir` to encourage the use of `TestContext::app_dir_preprocessor`. ([#431](https://github.com/heroku/libcnb.rs/pull/431)).
- Remove `TestContext::app_dir` to encourage the use of `TestConfig::app_dir_preprocessor`. ([#431](https://github.com/heroku/libcnb.rs/pull/431)).
- Improve performance when no `TestConfig::app_dir_preprocessor` is configured by skipping application directory copying. ([#431](https://github.com/heroku/libcnb.rs/pull/431)).

## [0.3.1] 2022-04-12

Expand Down
41 changes: 37 additions & 4 deletions libcnb-test/src/app.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use fs_extra::dir::CopyOptions;
use std::path::Path;
use std::ffi::OsStr;
use std::path::{Path, PathBuf};
use tempfile::{tempdir, TempDir};

/// Copies an application directory to a temporary location.
pub(crate) fn copy_app(app_dir: impl AsRef<Path>) -> Result<TempDir, PrepareAppError> {
pub(crate) fn copy_app(app_dir: impl AsRef<Path>) -> Result<AppDir, PrepareAppError> {
tempdir()
.map_err(PrepareAppError::CreateTempDirError)
.and_then(|temp_app_dir| {
Expand All @@ -16,7 +17,7 @@ pub(crate) fn copy_app(app_dir: impl AsRef<Path>) -> Result<TempDir, PrepareAppE
},
)
.map_err(PrepareAppError::CopyAppError)
.map(|_| temp_app_dir)
.map(|_| temp_app_dir.into())
})
}

Expand All @@ -26,6 +27,38 @@ pub enum PrepareAppError {
CopyAppError(fs_extra::error::Error),
}

pub(crate) enum AppDir {
Temporary(TempDir),
Unmanaged(PathBuf),
}

impl AppDir {
pub fn as_path(&self) -> &Path {
match self {
AppDir::Temporary(temp_dir) => temp_dir.path(),
AppDir::Unmanaged(path) => path,
}
}
}

impl AsRef<OsStr> for AppDir {
fn as_ref(&self) -> &OsStr {
self.as_path().as_os_str()
}
}

impl From<PathBuf> for AppDir {
fn from(value: PathBuf) -> Self {
AppDir::Unmanaged(value)
}
}

impl From<TempDir> for AppDir {
fn from(value: TempDir) -> Self {
AppDir::Temporary(value)
}
}

#[cfg(test)]
mod tests {
use std::collections::HashMap;
Expand Down Expand Up @@ -59,7 +92,7 @@ mod tests {
let temp_app_dir = super::copy_app(source_app_dir.path()).unwrap();

for (path, contents) in files {
let absolute_path = temp_app_dir.path().join(path);
let absolute_path = temp_app_dir.as_path().join(path);

assert_eq!(std::fs::read_to_string(absolute_path).unwrap(), contents);
}
Expand Down
36 changes: 22 additions & 14 deletions libcnb-test/src/test_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,21 +134,29 @@ impl TestRunner {
) {
let config = config.borrow();

let app_dir = if config.app_dir.is_relative() {
env::var("CARGO_MANIFEST_DIR")
.map(PathBuf::from)
.expect("Could not determine Cargo manifest directory")
.join(&config.app_dir)
} else {
config.app_dir.clone()
};
let app_dir = {
let normalized_app_dir_path = if config.app_dir.is_relative() {
env::var("CARGO_MANIFEST_DIR")
.map(PathBuf::from)
.expect("Could not determine Cargo manifest directory")
.join(&config.app_dir)
} else {
config.app_dir.clone()
};

let temp_app_dir =
app::copy_app(&app_dir).expect("Could not copy app to temporary location");
// Copy the app to a temporary directory if an app_dir_preprocessor is specified and run the
// preprocessor. Skip app copying if no changes to the app will be made.
if let Some(app_dir_preprocessor) = &config.app_dir_preprocessor {
let app_dir = app::copy_app(&normalized_app_dir_path)
.expect("Could not copy app to temporary location");

if let Some(app_dir_preprocessor) = &config.app_dir_preprocessor {
(app_dir_preprocessor)(PathBuf::from(temp_app_dir.path()));
}
(app_dir_preprocessor)(app_dir.as_path().to_owned());

app_dir
} else {
normalized_app_dir_path.into()
}
};

let temp_crate_buildpack_dir =
config
Expand All @@ -161,7 +169,7 @@ impl TestRunner {

let mut pack_command = PackBuildCommand::new(
&config.builder_name,
temp_app_dir.path(),
&app_dir,
&image_name,
// Prevent redundant image-pulling, which slows tests and risks hitting registry rate limits.
PullPolicy::IfNotPresent,
Expand Down

0 comments on commit debff59

Please sign in to comment.