Merge pull request #105 from github/kpaulisse-fix-ignore-tags

Remove legacy exclusion of tags

Author: kpaulisse

doc/advanced-dynamic-ignores.md

@@ -2,6 +2,8 @@
 
 Using the `--ignore-tags` command line option, it is possible to ignore all resources with particular Puppet tags. This allows dynamic ignoring of wrappers or other resources that are not of interest.
 
+NOTE: This option is separate and distinct from `--include-tags`, which controls whether differences in tags themselves will appear as a difference. For more on `--include-tags`, consult the [options reference](/doc/optionsref.md).
+
 ## Getting Started
 
 To use ignored tags, you first need to decide what the name of your tag will be. The standard is `ignored_octocatalog_diff`.

lib/octocatalog-diff/catalog-diff/differ.rb

@@ -266,11 +266,8 @@ def filter_and_cleanup(catalog_resources)
               hsh[k] = cleansed_param unless cleansed_param.nil? || cleansed_param.empty?
             elsif k == 'tags'
               # The order of tags is unimportant. Sort this array to avoid false diffs if order changes.
-              # Also if tags is empty, don't add. Most uses of catalog diff will want to ignore tags,
-              # and if you're ignoring tags you won't get here anyway. Also, don't add empty array of tags.
-              unless @opts[:ignore_tags]
-                hsh[k] = v.sort if v.is_a?(Array) && v.any?
-              end
+              # Also if tags is empty, don't add.
+              hsh[k] = v.sort if v.is_a?(Array) && v.any?
             elsif k == 'file' || k == 'line'
               # We don't care, for the purposes of catalog-diff, from which manifest and line this resource originated.
               # However, we may report this to the user, so we will keep it in here for now.

spec/octocatalog-diff/fixtures/catalogs/include-tags-new.json

@@ -0,0 +1,41 @@
+{
+  "document_type": "Catalog",
+  "data": {
+    "tags": ["settings"],
+    "name": "my.rspec.node",
+    "version": "production",
+    "environment": "production",
+    "resources": [
+      {
+        "type": "Stage",
+        "title": "main",
+        "tags": ["stage"],
+        "exported": false,
+        "parameters": {
+          "name": "main"
+        }
+      },
+      {
+        "type": "Class",
+        "title": "Settings",
+        "tags": ["class","settings"],
+        "exported": false
+      },
+      {
+        "type": "File",
+        "title": "/tmp/foo",
+        "tags": ["tag-one-new","tag-too","ignore-tag__file"],
+        "parameters": {
+          "content": "foofoo",
+          "owner": "root"
+        }
+      }
+    ],
+    "classes": [
+      "settings"
+    ]
+  },
+  "metadata": {
+    "api_version": 1
+  }
+}

spec/octocatalog-diff/fixtures/catalogs/include-tags-old.json

@@ -0,0 +1,41 @@
+{
+  "document_type": "Catalog",
+  "data": {
+    "tags": ["settings"],
+    "name": "my.rspec.node",
+    "version": "production",
+    "environment": "production",
+    "resources": [
+      {
+        "type": "Stage",
+        "title": "main",
+        "tags": ["stage"],
+        "exported": false,
+        "parameters": {
+          "name": "main"
+        }
+      },
+      {
+        "type": "Class",
+        "title": "Settings",
+        "tags": ["class","settings"],
+        "exported": false
+      },
+      {
+        "type": "File",
+        "title": "/tmp/foo",
+        "tags": ["tag-one","tag-two"],
+        "parameters": {
+          "content": "foofoo",
+          "owner": "root"
+        }
+      }
+    ],
+    "classes": [
+      "settings"
+    ]
+  },
+  "metadata": {
+    "api_version": 1
+  }
+}

spec/octocatalog-diff/integration/include_tags_spec.rb

@@ -0,0 +1,147 @@
+# frozen_string_literal: true
+
+require_relative 'integration_helper'
+require 'json'
+
+describe 'include-tags integration' do
+  let(:default_argv) do
+    [
+      '--from-catalog', OctocatalogDiff::Spec.fixture_path('catalogs/include-tags-old.json'),
+      '--to-catalog', OctocatalogDiff::Spec.fixture_path('catalogs/include-tags-new.json')
+    ]
+  end
+
+  context 'with --include-tags specified' do
+    let(:argv) do
+      default_argv.concat ['--include-tags']
+    end
+
+    let(:result) do
+      OctocatalogDiff::Integration.integration(
+        argv: argv
+      )
+    end
+
+    it 'should exit indicating success with differences' do
+      expect(result.exitcode).to eq(2)
+    end
+
+    it 'should contain representative tag differences' do
+      diffs = result.diffs
+      expect(diffs.size).to eq(1)
+      expect(diffs.first.change?).to eq(true)
+      expect(diffs.first.structure).to eq(['tags'])
+      expect(diffs.first.old_value).to eq(['tag-one', 'tag-two'])
+      expect(diffs.first.new_value).to eq(['ignore-tag__file', 'tag-one-new', 'tag-too'])
+    end
+  end
+
+  context 'with --no-include-tags specified' do
+    let(:argv) do
+      default_argv.concat ['--no-include-tags']
+    end
+
+    let(:result) do
+      OctocatalogDiff::Integration.integration(
+        argv: argv
+      )
+    end
+
+    it 'should exit indicating success with no differences' do
+      expect(result.exitcode).to eq(0)
+    end
+
+    it 'should contain representative tag differences' do
+      diffs = result.diffs
+      expect(diffs.size).to eq(0)
+    end
+  end
+
+  context 'with --include-tags not specified' do
+    let(:argv) do
+      default_argv
+    end
+
+    let(:result) do
+      OctocatalogDiff::Integration.integration(
+        argv: argv
+      )
+    end
+
+    it 'should exit indicating success with no differences' do
+      expect(result.exitcode).to eq(0)
+    end
+
+    it 'should contain representative tag differences' do
+      diffs = result.diffs
+      expect(diffs.size).to eq(0)
+    end
+  end
+
+  context 'with --ignore-tags and --include-tags specified' do
+    let(:argv) do
+      default_argv.concat ['--include-tags', '--ignore-tags', 'foo']
+    end
+
+    let(:result) do
+      OctocatalogDiff::Integration.integration(
+        argv: argv
+      )
+    end
+
+    it 'should exit indicating success with differences' do
+      expect(result.exitcode).to eq(2)
+    end
+
+    it 'should contain representative tag differences' do
+      diffs = result.diffs
+      expect(diffs.size).to eq(1)
+      expect(diffs.first.change?).to eq(true)
+      expect(diffs.first.structure).to eq(['tags'])
+      expect(diffs.first.old_value).to eq(['tag-one', 'tag-two'])
+      expect(diffs.first.new_value).to eq(['ignore-tag__file', 'tag-one-new', 'tag-too'])
+    end
+  end
+
+  context 'with --ignore-tags and --no-include-tags specified' do
+    let(:argv) do
+      default_argv.concat ['--no-include-tags', '--ignore-tags', 'foo']
+    end
+
+    let(:result) do
+      OctocatalogDiff::Integration.integration(
+        argv: argv
+      )
+    end
+
+    it 'should exit indicating success with no differences' do
+      expect(result.exitcode).to eq(0)
+    end
+
+    it 'should contain representative tag differences' do
+      diffs = result.diffs
+      expect(diffs.size).to eq(0)
+    end
+  end
+
+  context 'with matching --ignore-tags and --include-tags specified' do
+    let(:argv) do
+      default_argv.concat ['--include-tags', '--ignore-tags', 'ignore-tag']
+    end
+
+    let(:result) do
+      OctocatalogDiff::Integration.integration(
+        argv: argv
+      )
+    end
+
+    it 'should exit indicating success with no differences' do
+      expect(result.exitcode).to eq(0)
+    end
+
+    it 'should contain representative tag differences' do
+      diffs = result.diffs
+      expect(diffs.size).to eq(0)
+    end
+  end
+end