Skip to content

Commit

Permalink
Code review feedback
Browse files Browse the repository at this point in the history
* Renamed `LibCnbRs` to `WorkspaceBuildpack`
* Renamed `Crate` to `CurrentCrate`
* Updated CHANGELOG.md and doc example
  • Loading branch information
colincasey committed Sep 8, 2023
1 parent bb714d9 commit 3863a69
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 10 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `libcnb-data`:
- `ExecDProgramOutputKey`, `ProcessType`, `LayerName`, `BuildpackId` and `StackId` now implement `Ord` and `PartialOrd`. ([#658](https://github.com/heroku/libcnb.rs/pull/658))
- Add `generic::GenericMetadata` as a generic metadata type. Also makes it the default for `BuildpackDescriptor`, `SingleBuildpackDescriptor`, `MetaBuildpackDescriptor` and `LayerContentMetadata`. ([#664](https://github.com/heroku/libcnb.rs/pull/664))
- `libcnb-test`:
- Added the variant `WorkspaceBuildpack` to the `build_config::BuildpackReference` enum which allows any buildpack within the Rust workspace to be referenced for testing. ([#666](https://github.com/heroku/libcnb.rs/pull/666))
- Testing of composite buildpacks is now supported using the `WorkspaceBuildpack` variant - **Requires `pack` CLI version `>=0.30`**. ([#666](https://github.com/heroku/libcnb.rs/pull/666))

### Changed

- Renamed `libcnb-data`'s `Buildpackage` to `PackageDescriptor`. This required changes in many other names across multiple libcnb.rs crates for consistency. See the PR diff for details, but renames should be straightforward and unsurprising. ([#656](https://github.com/heroku/libcnb.rs/pull/656))
- `libcnb-cargo`:
- No longer outputs paths for non-libcnb.rs and non-meta buildpacks. ([#657](https://github.com/heroku/libcnb.rs/pull/657))
- Build output for humans changed slightly, output intended for machines/scripting didn't change. ([#657](https://github.com/heroku/libcnb.rs/pull/657))
- `libcnb-test`:
- Renamed the variant `Crate` to `CurrentCrate` for the `build_config::BuildpackReference` enum which references the buildpack within the Rust Crate currently being tested. ([#666](https://github.com/heroku/libcnb.rs/pull/666))

## [0.14.0] - 2023-08-18

Expand Down
4 changes: 3 additions & 1 deletion libcnb-test/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -191,13 +191,15 @@ fn dynamic_fixture() {
Building with multiple buildpacks, using [`BuildConfig::buildpacks`]:

```rust,no_run
use libcnb_data::buildpack_id;
use libcnb_test::{BuildConfig, BuildpackReference, TestRunner};
// #[test]
fn additional_buildpacks() {
TestRunner::default().build(
BuildConfig::new("heroku/builder:22", "test-fixtures/app").buildpacks(vec![
BuildpackReference::Crate,
BuildpackReference::CurrentCrate,
BuildpackReference::WorkspaceBuildpack(buildpack_id!("my-project/buildpack")),
BuildpackReference::Other(String::from("heroku/another-buildpack")),
]),
|context| {
Expand Down
10 changes: 5 additions & 5 deletions libcnb-test/src/build_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ impl BuildConfig {
cargo_profile: CargoProfile::Dev,
target_triple: String::from("x86_64-unknown-linux-musl"),
builder_name: builder_name.into(),
buildpacks: vec![BuildpackReference::Crate],
buildpacks: vec![BuildpackReference::CurrentCrate],
env: HashMap::new(),
app_dir_preprocessor: None,
expected_pack_result: PackResult::Success,
Expand All @@ -51,7 +51,7 @@ impl BuildConfig {

/// Sets the buildpacks (and their ordering) to use when building the app.
///
/// Defaults to [`BuildpackReference::Crate`].
/// Defaults to [`BuildpackReference::CurrentCrate`].
///
/// # Example
/// ```no_run
Expand All @@ -60,7 +60,7 @@ impl BuildConfig {
/// TestRunner::default().build(
/// BuildConfig::new("heroku/builder:22", "test-fixtures/app").buildpacks(vec![
/// BuildpackReference::Other(String::from("heroku/another-buildpack")),
/// BuildpackReference::Crate,
/// BuildpackReference::CurrentCrate,
/// ]),
/// |context| {
/// // ...
Expand Down Expand Up @@ -251,9 +251,9 @@ impl BuildConfig {
#[derive(Debug, Clone, Eq, PartialEq)]
pub enum BuildpackReference {
/// References the buildpack in the Rust Crate currently being tested.
Crate,
CurrentCrate,
/// References a libcnb.rs buildpack within the Rust Workspace that needs to be packaged into a buildpack
LibCnbRs(BuildpackId),
WorkspaceBuildpack(BuildpackId),
/// References another buildpack by id, local directory or tarball.
Other(String),
}
Expand Down
4 changes: 2 additions & 2 deletions libcnb-test/src/test_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,14 +98,14 @@ impl TestRunner {

for buildpack in &config.buildpacks {
match buildpack {
BuildpackReference::Crate => {
BuildpackReference::CurrentCrate => {
let crate_buildpack_dir = build::package_crate_buildpack(config.cargo_profile, &config.target_triple)
.expect("Test references crate buildpack, but crate wasn't packaged as a buildpack. This is an internal libcnb-test error, please report any occurrences");
pack_command.buildpack(crate_buildpack_dir.path.clone());
buildpack_dirs.push(crate_buildpack_dir);
}

BuildpackReference::LibCnbRs(builpack_id) => {
BuildpackReference::WorkspaceBuildpack(builpack_id) => {
let buildpack_dir = build::package_buildpack(builpack_id, config.cargo_profile, &config.target_triple)
.unwrap_or_else(|_| panic!("Test references buildpack `{builpack_id}`, but this directory wasn't packaged as a buildpack. This is an internal libcnb-test error, please report any occurrences"));
pack_command.buildpack(buildpack_dir.path.clone());
Expand Down
4 changes: 2 additions & 2 deletions libcnb-test/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ fn address_for_port_when_container_crashed() {
fn basic_build_with_libcnb_reference_to_single_buildpack() {
TestRunner::default().build(
BuildConfig::new("heroku/builder:22", "test-fixtures/procfile").buildpacks(vec![
BuildpackReference::LibCnbRs(buildpack_id!("libcnb-test/a")),
BuildpackReference::WorkspaceBuildpack(buildpack_id!("libcnb-test/a")),
]),
|context| {
assert_empty!(context.pack_stderr);
Expand All @@ -566,7 +566,7 @@ fn basic_build_with_libcnb_reference_to_single_buildpack() {
fn basic_build_with_libcnb_reference_to_meta_buildpack() {
TestRunner::default().build(
BuildConfig::new("heroku/builder:22", "test-fixtures/procfile").buildpacks(vec![
BuildpackReference::LibCnbRs(buildpack_id!("libcnb-test/meta")),
BuildpackReference::WorkspaceBuildpack(buildpack_id!("libcnb-test/meta")),
]),
|context| {
assert_empty!(context.pack_stderr);
Expand Down

0 comments on commit 3863a69

Please sign in to comment.