Command.Context() returns nil instead of context.Background()

[andreaangiolillo]

Closes #1578

[CLAassistant]

All committers have signed the CLA.

[johnSchnake]

Looks good to me 👍 Could you squash the commits down before I merge though?

[marckhouzam]

Out of curiosity, what specific scenario does this address?
I'm asking because the context is set to context.Background() once Execute() is called:

cobra/command.go

Lines 915 to 918 in 6d2dc43

	func (c *Command) ExecuteC() (cmd *Command, err error) {
	if c.ctx == nil {
	c.ctx = context.Background()
	}

[katexochen]

@marckhouzam I often use package injection in a command's Run function foo, which then passes the command to another function bar. In unit test, I just call func bar, pass a new command, and don't call Execute. If function bar uses a context from the command, it panics during the test.

func newFooCommand() *cobra.Command {
	return &cobra.Command{ RunE: foo }
}

func foo(cmd *cobra.Command, args []string) error {
	fs := afero.NewOsFs()
	return bar(cmd, args, fs)
}

func bar(cmd *cobra.Command, args []string, fs afero.Fs) error {

	// use cmd.Context

	return nil
}

func TestFoo(t *testing.T) {
	cmd := newFooCommand()
	fs := afero.NewMemMapFs()
	assert.NoError(t, bar(cmd, []string{}, fs))
}

[marckhouzam]

Now that we have a SetContext() function thanks to #1551, do we still need to set the context in Context()?

I'm just a little hesitant because maybe people will want to use nil as a value for a context in some cases by using SetContext()? But I admit I don't really have a good feel for this.

[katexochen]

do we still need to set the context in Context()?

From my perspective, #1551 should at least solve the issue I've described above (#1582 (comment)). However, if this PR isn't merged, the doc comment of the Command.Context method should be updated and state that nil is returned if no context has been set.

[marckhouzam]

After discussion we felt that there was more flexibility and less risk in allowing a nil context.

And now that we have a SetContext() function thanks to #1551 users can call SetContext() to set the context to Background() without having to call ExecuteWithContext().

The GoDoc was adjusted by #1639.

I'm therefore going to close this but if you still feel something is missing, feel free to re-open.