Skip to content

Commit

Permalink
Improve integration test coverage (#75)
Browse files Browse the repository at this point in the history
Whilst the unit tests for this repository are very comprehensive, much
of the end buildpack behaviour depends on Pack CLI / `lifecycle` / the
buildpack API version, which aren't tested by the unit tests.

For example Buildpack API 0.6 changed the default process handling in a
breaking way:
https://github.com/buildpacks/spec/releases/tag/buildpack%2Fv0.6

Now that `libcnb-test` has added support for more methods of invoking
the built containers (see heroku/libcnb.rs#366),
we are able to add integration tests for more than just a single `web`
process example. 

This PR:
- Adds five additional integration tests.
- Drops the usage of `ureq` / making HTTPS requests to the `web` process,
  in favour of a simpler `echo` / logs output based approach. The only
  reason the former approach was used was because of the "valid Procfile
  but not valid YAML" case (see 741bbc0),
  however that is now tested via a simpler `echo` fixture that includes
  the offending `"key: value"` substring, rather than  the `socat`
  example command.
- Drops usage of `tempfile` / manually writing files in the unit tests,
  in favour of reading files from the fixtures directory.
- Uses `indoc!` to assert against multi-line log output strings, to make
  the end output more clearly visible from the test, and also reduce the
  chance of false positives (for example, `Procfile declares types -> web`
  would previously successfully match against
  `Procfile declares types -> web, something-else` since there was no end
  of line marker).
- Uses the new `ignore = "reason"` support in Rust 1.61, to make it
  clearer why (integration) tests are being skipped when running
  `cargo test`.
- Remove the redundant `.buildpacks(vec![BuildpackReference::Crate])`
  calls, since the current buildpack is the default when using
  `libcnb-test`'s `run_test()`.
- Removes `RUST_BACKTRACE=1` from the Circle CI configs, since in
  general the backtraces are noisy (making it harder to read the actual
  error) and add little value over the file+line number that's already
  output in the standard error message. In rare scenarios where the
  backtrace is actually useful, it can be enabled locally, or on a
  Circle CI run with a debugging commit pushed to the PR etc.
- Adds `heroku/builder:22` to the trusted builders list, since otherwise
  it slows down builds, and also means log output has additional prefixes,
  which mess up the multi-line assertions. Longer term we will add this
  builder to Pack's built-in trusted builders list, and also make
  `libcnb-test` pass `--trust-builder` to the `pack build` command:
  heroku/libcnb.rs#407

GUS-W-11311966.
  • Loading branch information
edmorley authored Jun 20, 2022
1 parent 683de9b commit 9a9ba97
Show file tree
Hide file tree
Showing 13 changed files with 317 additions and 199 deletions.
6 changes: 5 additions & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,13 @@ jobs:
- run:
name: Add musl target
command: rustup target add x86_64-unknown-linux-musl
# This can be removed when https://github.com/heroku/libcnb.rs/issues/407 is fixed.
- run:
name: Mark heroku/builder:22 as a trusted builder
command: pack config trusted-builders add heroku/builder:22
- run:
name: Run tests
command: RUST_BACKTRACE=1 cargo test --all-features --locked -- --include-ignored
command: cargo test --all-features --locked -- --include-ignored
- save-cargo-cache

workflows:
Expand Down
Loading

0 comments on commit 9a9ba97

Please sign in to comment.