Skip to content

Conversation

lbajolet-hashicorp
Copy link
Contributor

Dumping a command's results for black-box tests used to involve creating a checker, adding it to the Assert list, and running the command.
This was a bit verbose, and forced us to forward the testing.T instance to the checker so it could output it.

This PR changes how dumping a test's results work, by adding a Dump() function to the packerCommand directly, so we lighten the changes needed to have information on the command when troubleshooting/writing a test for Packer using this method.

When running a black-box Packer test, crashes were to be detected
automatically so that tests that crash are always reported as a failure.

However, the match for detecting that Packer crashed was incorrectly
formed, so those weren't detected.
Previous versions of the packerCommand implementation for wrapping a
freshly compiled Packer binary to run tests with were a bit opaque, and
printing out the logs from a run was defined in `Assert` as a plain
checker, using a bit of a clumsy call that repeats already known
information like `testing.T`.

This commit changes the implementation so that `Dump` becomes a function
call akin to the others, which doesn't need any parameter, instead it
leverages what if already kept within the packerCommand structure.
In addition to dumping the results of a command, we also now dump the
command itself, so we know exactly which command was invoked to produce
the result we got.
@lbajolet-hashicorp lbajolet-hashicorp added the tech-debt Issues and pull requests related to addressing technical debt or improving the codebase label Mar 28, 2025
@lbajolet-hashicorp lbajolet-hashicorp requested a review from a team as a code owner March 28, 2025 14:38
Copy link
Collaborator

@mogrogan mogrogan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tech-debt Issues and pull requests related to addressing technical debt or improving the codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants