cmd/snapctl: allow snapctl -h without a context (regression fix). #3907
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3907 +/- ##
==========================================
+ Coverage 75.91% 75.91% +<.01%
==========================================
Files 416 416
Lines 35972 35969 -3
==========================================
- Hits 27308 27306 -2
+ Misses 6747 6746 -1
Partials 1917 1917
Continue to review full report at Codecov.
|
| } | ||
| // Ignore missing context error to allow 'snapctl -h' without a context; | ||
| // Actual context is validated later by get/set. | ||
| context, _ := c.d.overlord.HookManager().Context(snapctlOptions.ContextID) | ||
| stdout, stderr, err := ctlcmd.Run(context, snapctlOptions.Args) |
There was a problem hiding this comment.
This line would never get a nil context, but now it can. Are we sure it won't crash (panic) if the context is actually used (say, "snapctl get" out of hooks, rather than -h).
There was a problem hiding this comment.
Yes, it's fine, we check for nils early in get/set and we have unit tests for nil context for both get and set.
| @@ -11,6 +11,7 @@ execute: | | |||
| echo "Verify that snapctl -h runs without a context" | |||
| if ! snapctl -h; then | |||
| echo "Expected snapctl -h to be successful" | |||
| exit 1 | |||
There was a problem hiding this comment.
🤦♂️ I thought we had a test for it! Don't run a git blame.
There was a problem hiding this comment.
I won't, it stays between us ;)
Fix a subtle regression introduced with ephemeral contexts: snapctl -h wouldn't work anymore outside of hooks. Also fix the broken test that checks that.