Skip to content

Conversation

@ess
Copy link

@ess ess commented Feb 6, 2023

Per the desire expressed in the TODO in the default raw instruction processing case in DockerfileFromHistory, this adds tests for that processor and refactors it.

The end result increases cumulative cyclomatic complexity a few points, but generally drops the complexity of the individual pieces through conditional flattening and code deduplication, provides a clear path towards extension, and generally makes the default processor a good deal easier to reason about.

ess added 5 commits February 6, 2023 10:32
In order to test and refactor the default case for the raw
instruction processor, this change identify the values affected
(`inst`and `isExecForm`), and extracts the case body out to
a new function.

Signed-off-by: Dennis Walters <[email protected]>
Added unit tests for the function that was previously extracted
from `DockerfileFromHistory` to ensure refactored behavior.

Signed-off-by: Dennis Walters <[email protected]>
* Extracts `processAsUncaughtInst` for handling standard
  instructions that were given in a non-standard form (as
  may happen with buildkit, etc)
* Extracts `processAsArgsFormat` for handling RUN instructions
  that were provided in the args format
* Extracts `processAsExecRun` for handling commands that were
  provided in a non-standard way
* Extracts `detectRawShellForm` for detecting commands that
  are shaped like shell form RUN instructions
* Extracts `execify` for converting arrays of strings to
  exec format RUN instructions (and lowering code duplication)
* Replaces the body of `processRawInst` to incorporate these
  extracted functions and generally make the problem easier
  to reason about and extend

Signed-off-by: Dennis Walters <[email protected]>
Signed-off-by: Dennis Walters <[email protected]>
Signed-off-by: Dennis Walters <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant