Fix not to ignore diff selector when it matched nothing

Fixes #327

Signed-off-by: Yusuke Kuoka <ykuoka@gmail.com>

pkg/app/app.go

@@ -1866,12 +1866,12 @@ func (a *App) withNeeds(r *Run, c DAGConfig, includeDisabled bool, f func(*state
 
 	var toRender []state.ReleaseSpec
 
-	releasesDisabled := map[string]state.ReleaseSpec{}
+	releasesToUninstall := map[string]state.ReleaseSpec{}
 	for _, r := range selectedReleasesWithNeeds {
 		release := r
 		id := state.ReleaseToID(&release)
 		if release.Installed != nil && !*release.Installed {
-			releasesDisabled[id] = release
+			releasesToUninstall[id] = release
 		} else {
 			toRender = append(toRender, release)
 		}
@@ -1879,6 +1879,7 @@ func (a *App) withNeeds(r *Run, c DAGConfig, includeDisabled bool, f func(*state
 
 	var rels []state.ReleaseSpec
 
+	if len(toRender) > 0 {
 		// toRender already contains the direct and transitive needs depending on the DAG options.
 		// That's why we don't pass in `IncludeNeeds: c.IncludeNeeds(), IncludeTransitiveNeeds: c.IncludeTransitiveNeeds()` here.
 		// Otherwise, in case include-needs=true, it will include the needs of needs, which results in unexpectedly introducing transitive needs,
@@ -1889,9 +1890,10 @@ func (a *App) withNeeds(r *Run, c DAGConfig, includeDisabled bool, f func(*state
 		})); len(errs) > 0 {
 			return false, errs
 		}
+	}
 
 	if includeDisabled {
-		for _, d := range releasesDisabled {
+		for _, d := range releasesToUninstall {
 			rels = append(rels, d)
 		}
 	}

pkg/app/app_diff_test.go

@@ -321,3 +321,150 @@ releases:
 		})
 	})
 }
+
+func TestDiffWithInstalled(t *testing.T) {
+	type testcase struct {
+		helmfile  string
+		ns        string
+		error     string
+		selectors []string
+		lists     map[exectest.ListKey]string
+		diffed    []exectest.Release
+	}
+
+	check := func(t *testing.T, tc testcase) {
+		t.Helper()
+
+		wantDiffs := tc.diffed
+
+		var helm = &exectest.Helm{
+			FailOnUnexpectedList: true,
+			FailOnUnexpectedDiff: true,
+			Lists:                tc.lists,
+			DiffMutex:            &sync.Mutex{},
+			ChartsMutex:          &sync.Mutex{},
+			ReleasesMutex:        &sync.Mutex{},
+		}
+
+		bs := &bytes.Buffer{}
+
+		func() {
+			t.Helper()
+
+			logReader, logWriter := io.Pipe()
+
+			logFlushed := &sync.WaitGroup{}
+			// Ensure all the log is consumed into `bs` by calling `logWriter.Close()` followed by `logFlushed.Wait()`
+			logFlushed.Add(1)
+			go func() {
+				scanner := bufio.NewScanner(logReader)
+				for scanner.Scan() {
+					bs.Write(scanner.Bytes())
+					bs.WriteString("\n")
+				}
+				logFlushed.Done()
+			}()
+
+			defer func() {
+				// This is here to avoid data-trace on bytes buffer `bs` to capture logs
+				if err := logWriter.Close(); err != nil {
+					panic(err)
+				}
+				logFlushed.Wait()
+			}()
+
+			logger := helmexec.NewLogger(logWriter, "debug")
+
+			valsRuntime, err := vals.New(vals.Options{CacheSize: 32})
+			if err != nil {
+				t.Errorf("unexpected error creating vals runtime: %v", err)
+			}
+
+			files := map[string]string{
+				"/path/to/helmfile.yaml": tc.helmfile,
+			}
+
+			app := appWithFs(&App{
+				OverrideHelmBinary:  DefaultHelmBinary,
+				fs:                  filesystem.DefaultFileSystem(),
+				OverrideKubeContext: "default",
+				Env:                 "default",
+				Logger:              logger,
+				helms: map[helmKey]helmexec.Interface{
+					createHelmKey("helm", "default"): helm,
+				},
+				valsRuntime: valsRuntime,
+			}, files)
+
+			if tc.ns != "" {
+				app.Namespace = tc.ns
+			}
+
+			if tc.selectors != nil {
+				app.Selectors = tc.selectors
+			}
+
+			diffErr := app.Diff(applyConfig{
+				// if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic.
+				concurrency: 1,
+				logger:      logger,
+				skipNeeds:   true,
+			})
+
+			var gotErr string
+			if diffErr != nil {
+				gotErr = diffErr.Error()
+			}
+
+			if d := cmp.Diff(tc.error, gotErr); d != "" {
+				t.Fatalf("unexpected error: want (-), got (+): %s", d)
+			}
+
+			require.Equal(t, wantDiffs, helm.Diffed)
+		}()
+
+		testhelper.RequireLog(t, "app_diff_test", bs)
+	}
+
+	t.Run("show diff on changed selected release", func(t *testing.T) {
+		check(t, testcase{
+			helmfile: `
+releases:
+- name: a
+  chart: incubator/raw
+  namespace: default
+- name: b
+  chart: incubator/raw
+  namespace: default
+`,
+			selectors: []string{"name=a"},
+			lists: map[exectest.ListKey]string{
+				{Filter: "^a$", Flags: helmV2ListFlags}: `NAME	REVISION	UPDATED                 	STATUS  	CHART        	APP VERSION	NAMESPACE
+foo 	4       	Fri Nov  1 08:40:07 2019	DEPLOYED	raw-3.1.0	3.1.0      	default
+`,
+			},
+			diffed: []exectest.Release{
+				{Name: "a", Flags: []string{"--kube-context", "default", "--namespace", "default"}},
+			},
+		})
+	})
+
+	t.Run("shows no diff on already uninstalled selected release", func(t *testing.T) {
+		check(t, testcase{
+			helmfile: `
+releases:
+- name: a
+  chart: incubator/raw
+  installed: false
+  namespace: default
+- name: b
+  chart: incubator/raw
+  namespace: default
+`,
+			selectors: []string{"name=a"},
+			lists: map[exectest.ListKey]string{
+				{Filter: "^a$", Flags: helmV2ListFlags}: ``,
+			},
+		})
+	})
+}

pkg/app/testdata/app_diff_test/show_diff_on_changed_selected_release

@@ -0,0 +1,40 @@
+processing file "helmfile.yaml" in directory "."
+changing working directory to "/path/to"
+first-pass rendering starting for "helmfile.yaml.part.0": inherited=&{default map[] map[]}, overrode=<nil>
+first-pass uses: &{default map[] map[]}
+first-pass rendering output of "helmfile.yaml.part.0":
+ 0: 
+ 1: releases:
+ 2: - name: a
+ 3:   chart: incubator/raw
+ 4:   namespace: default
+ 5: - name: b
+ 6:   chart: incubator/raw
+ 7:   namespace: default
+ 8: 
+
+first-pass produced: &{default map[] map[]}
+first-pass rendering result of "helmfile.yaml.part.0": {default map[] map[]}
+vals:
+map[]
+defaultVals:[]
+second-pass rendering result of "helmfile.yaml.part.0":
+ 0: 
+ 1: releases:
+ 2: - name: a
+ 3:   chart: incubator/raw
+ 4:   namespace: default
+ 5: - name: b
+ 6:   chart: incubator/raw
+ 7:   namespace: default
+ 8: 
+
+merged environment: &{default map[] map[]}
+1 release(s) matching name=a found in helmfile.yaml
+
+processing 1 groups of releases in this order:
+GROUP RELEASES
+1     default/default/a
+
+processing releases in group 1/1: default/default/a
+changing working directory back to "/path/to"

pkg/app/testdata/app_diff_test/shows_no_diff_on_already_uninstalled_selected_release

@@ -0,0 +1,37 @@
+processing file "helmfile.yaml" in directory "."
+changing working directory to "/path/to"
+first-pass rendering starting for "helmfile.yaml.part.0": inherited=&{default map[] map[]}, overrode=<nil>
+first-pass uses: &{default map[] map[]}
+first-pass rendering output of "helmfile.yaml.part.0":
+ 0: 
+ 1: releases:
+ 2: - name: a
+ 3:   chart: incubator/raw
+ 4:   installed: false
+ 5:   namespace: default
+ 6: - name: b
+ 7:   chart: incubator/raw
+ 8:   namespace: default
+ 9: 
+
+first-pass produced: &{default map[] map[]}
+first-pass rendering result of "helmfile.yaml.part.0": {default map[] map[]}
+vals:
+map[]
+defaultVals:[]
+second-pass rendering result of "helmfile.yaml.part.0":
+ 0: 
+ 1: releases:
+ 2: - name: a
+ 3:   chart: incubator/raw
+ 4:   installed: false
+ 5:   namespace: default
+ 6: - name: b
+ 7:   chart: incubator/raw
+ 8:   namespace: default
+ 9: 
+
+merged environment: &{default map[] map[]}
+1 release(s) matching name=a found in helmfile.yaml
+
+changing working directory back to "/path/to"