Hotfix: Skip Checkout Regression

[JustinGrote]

The testing logic for isLocalCheckout was incorrect, fix and add a test specifically for checkout.

Fixes #678

[mergify]

@JustinGrote this pull request has failed checks 🛠

[catthehacker]

Can we remove .secrets and .env loading from tests as well?

[mergify]

@JustinGrote this pull request has failed checks 🛠

[mergify]

@JustinGrote this pull request has failed checks 🛠

[JustinGrote]

Can we remove .secrets and .env loading from tests as well?

does that need to be done in the context of this PR or should it be done separately? I should have just fixed the last test and this should "go green" on this commit.

[mergify]

@JustinGrote this pull request has failed checks 🛠

[JustinGrote]

@JustinGrote this pull request has failed checks 🛠
This is probably related to why I put the invalid step check in the first place, let me nail it down.

[catthehacker]

Can we remove .secrets and .env loading from tests as well?

does that need to be done in the context of this PR or should it be done separately? I should have just fixed the last test and this should "go green" on this commit.

It was added because of that specific issue so I would prefer it to be gone otherwise it will mess with testing.

[JustinGrote]

Sure, I can regress that as well, but it will still show up if you, for instance, want to check out a specific ref as part of your flow.

[catthehacker]

Sure, I can regress that as well, but it will still show up if you, for instance, want to check out a specific ref as part of your flow.

Sounds like we need more test cases 😼

[JustinGrote]

Sure, I can regress that as well, but it will still show up if you, for instance, want to check out a specific ref as part of your flow.

Sounds like we need more test cases 😼

Probably :)

The current kernel panic is in the empty workflow case, it's generating a nil remote action that when it hits IsCheckout() causes a nil reference panic. It looks like the old flow was to get past this and drop an error later, sorting that out.

[codecov]

Codecov Report

Merging #680 (5ab38fa) into master (0f04942) will increase coverage by 1.26%.
The diff coverage is 61.61%.

@@            Coverage Diff             @@
##           master     #680      +/-   ##
==========================================
+ Coverage   49.27%   50.53%   +1.26%     
==========================================
  Files          23       23              
  Lines        2401     2525     +124     
==========================================
+ Hits         1183     1276      +93     
- Misses       1090     1113      +23     
- Partials      128      136       +8     

Impacted Files	Coverage Δ
pkg/container/docker_run.go	1.82% <0.00%> (-0.11%)	⬇️
pkg/model/workflow.go	30.71% <33.33%> (+5.00%)	⬆️
pkg/common/git.go	55.12% <33.92%> (-4.68%)	⬇️
pkg/model/planner.go	34.56% <41.37%> (+1.48%)	⬆️
pkg/container/docker_pull.go	36.17% <64.70%> (+17.98%)	⬆️
pkg/runner/step_context.go	73.00% <74.79%> (+4.05%)	⬆️
pkg/runner/run_context.go	79.74% <94.82%> (+3.33%)	⬆️
pkg/runner/runner.go	76.92% <100.00%> (+0.45%)	⬆️
... and 1 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aa68181...5ab38fa. Read the comment docs.

[JustinGrote]

@catthehacker fixed and the .secrets regressed as well, LGTM

[ayozemr]

Thanks for this!! Will this be part of a release anytime soon?