Fix empty diff being returned when single *cached* step had to be executed
Created by: mrnugget
This PR fixes a bug in the step-wise caching logic that @eseliger and I ran into last week.
Problem
When no cached result for the complete task was found, but a cached result for a single step and that step was the only left to execute then an empty diff was returned.
How to reproduce
-
Execute a batch spec with the following steps
steps: - run: echo "this is step 1" >> README.txt container: alpine:3 - run: echo "this is step 2" >> README.md container: alpine:3
the complete results and the results for each step are now cached.
-
Update the batch spec and re-execute:
steps: - run: echo "this is step 1" >> README.txt container: alpine:3
This will produce an empty diff because the Coordinator
did not find a cached result for the complete task/batch spec, but for the first step.
runSteps
didn't handle this case though, when only a single step had to be executed but that was also cached.
The fix
Take a look at my comments in the diffs here in GitHub to see what the fix is. Spoiler: it's just a condition and an early exit to handle the case of "cached step result == the only step to execute".
In order to get to this fix though I wrote a lot of tests, found one other bug (Files
was not cached), and cleaned up the 300 line long runSteps
function.
runSteps
is now much easier to understand and doesn't contain all levels of abstraction. The outer runSteps
concerns itself with which steps need to be executed and how to handle their results and executeSingleStep
does the lower-level work of rendering templates, starting containers, setting up the logger, etc.