Don't automatically request size if --size was explicitly set to false

[laurazard]

- What I did

Check if the --size flag was explicitly set to false (with --size=false) and don't automatically request the size when that's the case.

Also changed the flow a bit to always validate the template when --format is used.

Provides a workaround for docker/for-linux#1179

- How I did it

With Neovim 👀

- How to verify it

Run docker ps --format={{json .}} --size=false and check that size isn't calculated

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

[laurazard]

/cc @thaJeztah

[thaJeztah]

Oh! Was looking if we could update one of the unit tests to test this, but forgot to post; looks like we have some test for testing the auto-detect;

cli/cli/command/container/list_test.go

Lines 233 to 243 in cb5463a

	func TestContainerListFormatSizeSetsOption(t *testing.T) {
	cli := test.NewFakeCli(&fakeClient{
	containerListFunc: func(options types.ContainerListOptions) ([]types.Container, error) {
	assert.Check(t, options.Size)
	return []types.Container{}, nil
	},
	})
	cmd := newListCommand(cli)
	cmd.Flags().Set("format", `{{.Size}}`)
	assert.NilError(t, cmd.Execute())
	}

I guess we could make that a test-table to test the various options; I gave that a quick go;

func TestContainerListFormatSizeSetsOption(t *testing.T) {
	tests := []struct {
		doc, format, sizeFlag string
		sizeExpected          bool
	}{
		{
			doc:          "detect with all fields",
			format:       `{{json .}}`,
			sizeExpected: true,
		},
		{
			doc:          "detect with explicit field",
			format:       `{{.Size}}`,
			sizeExpected: true,
		},
		{
			doc:          "detect no size",
			format:       `{{.Names}}`,
			sizeExpected: false,
		},
		{
			doc:          "override enable",
			format:       `{{.Names}}`,
			sizeFlag:     "true",
			sizeExpected: true,
		},
		{
			doc:          "override disable",
			format:       `{{.Size}}`,
			sizeFlag:     "false",
			sizeExpected: false,
		},
	}

	for _, tc := range tests {
		tc := tc
		t.Run(tc.doc, func(t *testing.T) {
			cli := test.NewFakeCli(&fakeClient{
				containerListFunc: func(options types.ContainerListOptions) ([]types.Container, error) {
					assert.Check(t, is.Equal(options.Size, tc.sizeExpected))
					return []types.Container{}, nil
				},
			})
			cmd := newListCommand(cli)
			cmd.Flags().Set("format", tc.format)
			if tc.sizeFlag != "" {
				cmd.Flags().Set("size", tc.sizeFlag)
			}
			assert.NilError(t, cmd.Execute())
		})
	}
}

[laurazard]

That's great! I thought about adding a little test case and ended up forgetting about it, but this is more comprehensive. I'll add it in.

[codecov-commenter]

Codecov Report

Merging #4067 (3b09b75) into master (4a500f6) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

❗ Current head 3b09b75 differs from pull request most recent head 9733334. Consider uploading reports for the commit 9733334 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4067      +/-   ##
==========================================
- Coverage   59.20%   59.17%   -0.04%     
==========================================
  Files         287      287              
  Lines       24698    24731      +33     
==========================================
+ Hits        14622    14634      +12     
- Misses       9192     9212      +20     
- Partials      884      885       +1     

[thaJeztah]

Thanks! Probably fine to squash those commits to keep the changes together

[thaJeztah]

LGTM!

I added the docs-revisit label; we need to look if the documentation shows examples for this, and perhaps have an example / mention that this option can be used to explicitly disable the size-calculation.