Refactor lifecycle execution and tests

[natalieparellano]

Summary

• Don't pass anything in l.opts as function arguments (makes things simpler)
• Remove duplication in tests by moving configurable inputs to top-level vars

Hopefully this should make this part of the code base a bit easier to understand and make changes to.

Documentation

• Should this change be documented?
  • Yes, see #___
  • No

[natalieparellano]

The diff is quite large 😞 but it should be pretty viewable given that it's mostly code deletions (make sure to hide whitespace)

lifecycle_execution_test.go is still crying out for improvement (could be made a table test potentially) but this is a step in the right direction

[codecov]

Codecov Report

Merging #1539 (02c3f42) into main (495ef2a) will increase coverage by 0.02%.
The diff coverage is 89.03%.

❗ Current head 02c3f42 differs from pull request most recent head 96eaa06. Consider uploading reports for the commit 96eaa06 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1539      +/-   ##
==========================================
+ Coverage   80.11%   80.13%   +0.02%     
==========================================
  Files         155      155              
  Lines       10095    10090       -5     
==========================================
- Hits         8087     8085       -2     
+ Misses       1524     1522       -2     
+ Partials      484      483       -1     

Flag	Coverage Δ
os_linux	79.91% <89.03%> (+0.02%)	⬆️
os_macos	77.39% <89.03%> (+0.05%)	⬆️
os_windows	80.01% <89.03%> (+0.02%)	⬆️
unit	80.13% <89.03%> (+0.02%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

[natalieparellano]

I think I uncovered bugs as part of this work:

• For analyze phase, when gid is provided all other flags are overridden:

  pack/internal/build/lifecycle_execution.go

  Lines 409 to 411 in 495ef2a

  	if l.opts.GID >= overrideGID {
  	flagsOpt = WithFlags("-gid", strconv.Itoa(l.opts.GID))
  	}

• For analyze phase, -cache-dir <cache> is still provided when using a cache image:

  pack/internal/build/lifecycle_execution.go

  Line 381 in 495ef2a

  args = append([]string{"-cache-dir", l.mountPaths.cacheDir()}, args...)

  and

  pack/internal/build/lifecycle_execution.go

  Line 401 in 495ef2a

  flagsOpt = WithFlags("-cache-image", buildCache.Name())

but I think it makes sense to fix them separately so as to not leave any doubt that no logic was changed

[jjbustamante]

I think I uncovered bugs as part of this work:

• For analyze phase, when gid is provided all other flags are overridden:

  pack/internal/build/lifecycle_execution.go

  Lines 409 to 411 in 495ef2a

  	if l.opts.GID >= overrideGID {
  	flagsOpt = WithFlags("-gid", strconv.Itoa(l.opts.GID))
  	}

• For analyze phase, -cache-dir <cache> is still provided when using a cache image:

  pack/internal/build/lifecycle_execution.go

  Line 381 in 495ef2a

  args = append([]string{"-cache-dir", l.mountPaths.cacheDir()}, args...)

  
  and

  pack/internal/build/lifecycle_execution.go

  Line 401 in 495ef2a

  flagsOpt = WithFlags("-cache-image", buildCache.Name())

but I think it makes sense to fix them separately so as to not leave any doubt that no logic was changed

I agree, we can add coverage later

[jjbustamante]

I ❤️ this refactoring! Just fix the test

[dfreilich]

Great work! I really like the simplification of the call api

[dfreilich]

Thank you for doing this! I had tried, and then dropped the ball with this: #1522