Merge pull request #110 from riscv/106-107-108-fix-several-small-XLSX-norm-rule-issues

Fixes and unit-level tests for issues 106, 107, & 108

Author: james-ball-qualcomm

Makefile

@@ -28,30 +28,32 @@ TAGS_BACKEND := tags.rb
 CREATE_NORM_RULE_TOOL := create_normative_rules.rb
 
 # Input and output file names
-GOOD_TEST_ADOC_INPUT_FNAME := test.adoc
-BAD_TEST_ADOC_INPUT_FNAME := duplicate.adoc
-GOOD_NORM_TAGS_OUTPUT_FNAME := test-norm-tags.json
-BAD_NORM_TAGS_OUTPUT_FNAME := bad-tags.json
+MAIN_TEST_ADOC_INPUT_FNAME := test.adoc
+DUPLICATE_TEST_ADOC_INPUT_FNAME := duplicate.adoc
+MAIN_NORM_TAGS_OUTPUT_FNAME := test-norm-tags.json
+DUPLICATE_NORM_TAGS_OUTPUT_FNAME := duplicate-tags.json
 NORM_RULE_JSON_OUTPUT_FNAME := test-norm-rules.json
 NORM_RULE_XLSX_OUTPUT_FNAME := test-norm-rules.xlsx
+NORM_RULE_TAGS_NO_RULES_OUTPUT_FNAME := test-norm-rules_tags_no_rules.json
 
 # Built output files
-BUILT_NORM_TAGS_GOOD := $(BUILD_DIR)/$(GOOD_NORM_TAGS_OUTPUT_FNAME)
-BUILT_NORM_TAGS_BAD := $(BUILD_DIR)/$(BAD_NORM_TAGS_OUTPUT_FNAME)
+BUILT_NORM_TAGS_MAIN := $(BUILD_DIR)/$(MAIN_NORM_TAGS_OUTPUT_FNAME)
+BUILT_NORM_TAGS_DUPLICATE := $(BUILD_DIR)/$(DUPLICATE_NORM_TAGS_OUTPUT_FNAME)
 BUILT_NORM_RULES_JSON := $(BUILD_DIR)/$(NORM_RULE_JSON_OUTPUT_FNAME)
 BUILT_NORM_RULES_XLSX := $(BUILD_DIR)/$(NORM_RULE_XLSX_OUTPUT_FNAME)
+BUILT_NORM_RULES_TAGS_NO_RULES := $(BUILD_DIR)/$(NORM_RULE_TAGS_NO_RULES_OUTPUT_FNAME)
 
 # Copies of expected output files.
 # Use make target "update-expected" to update from build dir contents.
-EXPECTED_NORM_TAGS := $(NORM_RULE_EXPECTED_DIR)/$(GOOD_NORM_TAGS_OUTPUT_FNAME)
+EXPECTED_NORM_TAGS := $(NORM_RULE_EXPECTED_DIR)/$(MAIN_NORM_TAGS_OUTPUT_FNAME)
 EXPECTED_NORM_RULES_JSON := $(NORM_RULE_EXPECTED_DIR)/$(NORM_RULE_JSON_OUTPUT_FNAME)
 EXPECTED_NORM_RULES_XLSX := $(NORM_RULE_EXPECTED_DIR)/$(NORM_RULE_XLSX_OUTPUT_FNAME)
 
 # All normative rule definition input YAML files
 NORM_RULE_DEF_FILES := $(wildcard $(NORM_RULE_DEF_DIR)/*.yaml)
 
 # Add -t to each normative tag input filename and add prefix of "/" to make into absolute pathname.
-NORM_TAG_FILE_ARGS := $(foreach relative_pname,$(BUILT_NORM_TAGS_GOOD),-t /$(relative_pname))
+NORM_TAG_FILE_ARGS := $(foreach relative_pname,$(BUILT_NORM_TAGS_MAIN),-t /$(relative_pname))
 
 # Add -d to each normative rule definition filename
 NORM_RULE_DEF_ARGS := $(foreach relative_pname,$(NORM_RULE_DEF_FILES),-d $(relative_pname))
@@ -122,18 +124,19 @@ test: build-tests compare-tests
 
 # Build tests
 .PHONY: build-tests build-test-tags build-test-norm-rules-json build-test-norm-rules-xlsx
-build-tests: build-test-tags build-test-norm-rules-json build-test-norm-rules-xlsx
-build-test-tags: $(BUILT_NORM_TAGS_GOOD) $(BUILT_NORM_TAGS_BAD)
+build-tests: build-test-tags build-test-norm-rules-json build-test-norm-rules-xlsx build-test-tags-without-rules
+build-test-tags: $(BUILT_NORM_TAGS_MAIN) $(BUILT_NORM_TAGS_DUPLICATE)
 build-test-norm-rules-json: $(BUILT_NORM_RULES_JSON)
 build-test-norm-rules-xlsx: $(BUILT_NORM_RULES_XLSX)
+build-test-tags-without-rules: $(BUILT_NORM_RULES_TAGS_NO_RULES)
 
 # Compare tests against expected
 .PHONY: compare-tests
 compare-tests: compare-test-tags compare-test-norm-rules-json
 
-compare-test-tags: $(EXPECTED_NORM_TAGS) $(BUILT_NORM_TAGS_GOOD)
+compare-test-tags: $(EXPECTED_NORM_TAGS) $(BUILT_NORM_TAGS_MAIN)
 	@echo "CHECKING BUILT TAGS AGAINST EXPECTED TAGS"
-	diff $(EXPECTED_NORM_TAGS) $(BUILT_NORM_TAGS_GOOD) && echo "diff PASSED" || (echo "diff FAILED"; exit 1)
+	diff $(EXPECTED_NORM_TAGS) $(BUILT_NORM_TAGS_MAIN) && echo "diff PASSED" || (echo "diff FAILED"; exit 1)
 
 compare-test-norm-rules: $(EXPECTED_NORM_RULES) $(BUILT_NORM_RULES)
 
@@ -145,42 +148,51 @@ compare-test-norm-rules-json: $(EXPECTED_NORM_RULES_JSON) $(BUILT_NORM_RULES_JSO
 .PHONY: update-expected
 update-expected: update-test-tags update-test-norm-rules-json update-test-norm-rules-xlsx
 
-update-test-tags: $(BUILT_NORM_TAGS_GOOD)
-	cp -f $(BUILT_NORM_TAGS_GOOD) $(EXPECTED_NORM_TAGS)
+update-test-tags: $(BUILT_NORM_TAGS_MAIN)
+	cp -f $(BUILT_NORM_TAGS_MAIN) $(EXPECTED_NORM_TAGS)
 
 update-test-norm-rules-json: $(BUILT_NORM_RULES_JSON)
 	cp -f $(BUILT_NORM_RULES_JSON) $(EXPECTED_NORM_RULES_JSON)
 
 update-test-norm-rules-xlsx: $(BUILT_NORM_RULES_XLSX)
 	cp -f $(BUILT_NORM_RULES_XLSX) $(EXPECTED_NORM_RULES_XLSX)
 
-# Build normative tags with good adoc input
-$(BUILT_NORM_TAGS_GOOD): $(NORM_RULE_TESTS_DIR)/$(GOOD_TEST_ADOC_INPUT_FNAME) $(CONVERTERS_DIR)/$(TAGS_BACKEND)
+# Build normative tags with main adoc input
+$(BUILT_NORM_TAGS_MAIN): $(NORM_RULE_TESTS_DIR)/$(MAIN_TEST_ADOC_INPUT_FNAME) $(CONVERTERS_DIR)/$(TAGS_BACKEND)
 	$(WORKDIR_SETUP)
 	$(DOCKER_CMD) $(DOCKER_QUOTE) $(ASCIIDOCTOR_TAGS) $(OPTIONS) -a tags-match-prefix='norm:' -a tags-output-suffix='-norm-tags.json' $< $(DOCKER_QUOTE)
 	$(WORKDIR_TEARDOWN)
 
-# Build normative tags with bad adoc input
+# Build normative tags with duplicate adoc input
 # Asciidoctor should exit with a non-zero status and then we just "touch" the output file so it exists and make is happy.
-$(BUILT_NORM_TAGS_BAD): $(TAGS_TESTS_DIR)/$(BAD_TEST_ADOC_INPUT_FNAME) $(CONVERTERS_DIR)/$(TAGS_BACKEND)
+$(BUILT_NORM_TAGS_DUPLICATE): $(TAGS_TESTS_DIR)/$(DUPLICATE_TEST_ADOC_INPUT_FNAME) $(CONVERTERS_DIR)/$(TAGS_BACKEND)
 	$(WORKDIR_SETUP)
-	$(DOCKER_CMD) $(DOCKER_QUOTE) $(ASCIIDOCTOR_TAGS) $(OPTIONS) -a tags-match-prefix='bad:' -a tags-output-suffix='-bad-tags.json' $< || touch build/bad-tags.json $(DOCKER_QUOTE)
+	$(DOCKER_CMD) $(DOCKER_QUOTE) $(ASCIIDOCTOR_TAGS) $(OPTIONS) -a tags-match-prefix='duplicate:' -a tags-output-suffix='-duplicate-tags.json' $< || touch $(BUILT_NORM_TAGS_DUPLICATE) $(DOCKER_QUOTE)
 	$(WORKDIR_TEARDOWN)
 
 # Build normative rules with JSON output format
-$(BUILT_NORM_RULES_JSON): $(BUILT_NORM_TAGS_GOOD) $(NORM_RULE_DEF_FILES)
+$(BUILT_NORM_RULES_JSON): $(BUILT_NORM_TAGS_MAIN) $(NORM_RULE_DEF_FILES)
 	$(WORKDIR_SETUP)
-	cp -f $(BUILT_NORM_TAGS_GOOD) $@.workdir
+	cp -f $(BUILT_NORM_TAGS_MAIN) $@.workdir
 	mkdir -p $@.workdir/build
-	$(DOCKER_CMD) $(DOCKER_QUOTE) ruby $(TOOLS_DIR)/$(CREATE_NORM_RULE_TOOL) $(NORM_TAG_FILE_ARGS) $(NORM_RULE_DEF_ARGS) $@ $(DOCKER_QUOTE)
+	$(DOCKER_CMD) $(DOCKER_QUOTE) ruby $(TOOLS_DIR)/$(CREATE_NORM_RULE_TOOL) -w $(NORM_TAG_FILE_ARGS) $(NORM_RULE_DEF_ARGS) $@ $(DOCKER_QUOTE)
 	$(WORKDIR_TEARDOWN)
 
 # Build normative rules with XLSX output format
-$(BUILT_NORM_RULES_XLSX): $(BUILT_NORM_TAGS_GOOD) $(NORM_RULE_DEF_FILES)
+$(BUILT_NORM_RULES_XLSX): $(BUILT_NORM_TAGS_MAIN) $(NORM_RULE_DEF_FILES)
 	$(WORKDIR_SETUP)
-	cp -f $(BUILT_NORM_TAGS_GOOD) $@.workdir
+	cp -f $(BUILT_NORM_TAGS_MAIN) $@.workdir
 	mkdir -p $@.workdir/build
-	$(DOCKER_CMD) $(DOCKER_QUOTE) ruby $(TOOLS_DIR)/$(CREATE_NORM_RULE_TOOL) -x $(NORM_TAG_FILE_ARGS) $(NORM_RULE_DEF_ARGS) $@ $(DOCKER_QUOTE)
+	$(DOCKER_CMD) $(DOCKER_QUOTE) ruby $(TOOLS_DIR)/$(CREATE_NORM_RULE_TOOL) -w -x $(NORM_TAG_FILE_ARGS) $(NORM_RULE_DEF_ARGS) $@ $(DOCKER_QUOTE)
+	$(WORKDIR_TEARDOWN)
+
+# Build normative rules that should create an error due to tags without norm rules referencing them
+# Should exit with a non-zero status and then we just "touch" the output file so it exists and make is happy.
+$(BUILT_NORM_RULES_TAGS_NO_RULES): $(BUILT_NORM_TAGS_MAIN) $(NORM_RULE_DEF_FILES)
+	$(WORKDIR_SETUP)
+	cp -f $(BUILT_NORM_TAGS_MAIN) $@.workdir
+	mkdir -p $@.workdir/build
+	$(DOCKER_CMD) $(DOCKER_QUOTE) ruby $(TOOLS_DIR)/$(CREATE_NORM_RULE_TOOL) $(NORM_TAG_FILE_ARGS) $(NORM_RULE_DEF_ARGS) $(BUILD_DIR)/bogus || touch $(BUILT_NORM_RULES_TAGS_NO_RULES) $(DOCKER_QUOTE)
 	$(WORKDIR_TEARDOWN)
 
 # Update docker image to latest

tests/norm-rule/expected/test-norm-rules.json

@@ -55,6 +55,18 @@
         }
       ]
     },
+    {
+      "name": "formulae",
+      "def_filename": "tests/norm-rule/test.yaml",
+      "chapter_name": "my-chapter_name",
+      "tags": [
+        {
+          "name": "norm:formulae",
+          "text": "This paragraph looks like a formulae to Excel because it has this < sign in it. Make sure this gets written as a string, not a formulae in the XLSX or else it will create an error in Excel.",
+          "tag_filename": "/build/test-norm-tags.json"
+        }
+      ]
+    },
     {
       "name": "rule_with_newlines",
       "def_filename": "tests/norm-rule/test.yaml",

tests/norm-rule/expected/test-norm-rules.xlsx

@@ -6,6 +6,7 @@
     "norm:paragraph:inline-anchors:entire": "Paragraph with inline anchor and something.",
     "norm:paragraph:tag_with_newlines": "Here’s the first line. Here’s the second line.",
     "norm:def": "This paragraph has two tags but we only ever get a tag for norm:def.",
+    "norm:formulae": "This paragraph looks like a formulae to Excel because it has this < sign in it. Make sure this gets written as a string, not a formulae in the XLSX or else it will create an error in Excel.",
     "norm:table:no-anchors-in-cells:entire-table": "Header 1|Header 2\n===\nCell in column 1, row 1|Cell in column 2, row 1\nCell in column 1, row 2|Cell in column 2, row 2\n===",
     "norm:table:anchors-in-cells:cell": "cell with anchor",
     "norm:table:anchors-in-cells:entire-table": "===\ncell with anchor\ncell without anchor\n===",
@@ -55,7 +56,8 @@
               "norm:paragraph:inline-anchors:inline-anchor",
               "norm:paragraph:inline-anchors:entire",
               "norm:paragraph:tag_with_newlines",
-              "norm:def"
+              "norm:def",
+              "norm:formulae"
             ]
           },
           {

tests/norm-rule/expected/test-norm-tags.json

@@ -51,6 +51,10 @@ This paragraph has two tags but we only ever get a tag for norm:def.
 [[def]]
 This paragraph has two tags but we'll never see either since the 1st isn't picked up by the tags backend as a tag and the second doesn't start with norm:
 
+[[norm:formulae]]
+This paragraph looks like a formulae to Excel because it has this < sign in it.
+Make sure this gets written as a string, not a formulae in the XLSX or else it will create an error in Excel.
+
 === Chapter 2 - Table Tagging
 
 // FAILS - Tag includes entire table but has adoc === prefixes/suffixes.

tests/norm-rule/test.adoc

@@ -21,6 +21,8 @@ normative_rule_definitions:
     tags: ["norm:paragraph:inline-anchors:entire"]
   - name: inline-anchors-in-paragraph-inline-anchor
     tags: ["norm:paragraph:inline-anchors:inline-anchor"]
+  - name: formulae
+    tags: ["norm:formulae"]
   - name: rule_with_newlines
     tags: ["norm:paragraph:tag_with_newlines"]
   - name: table1

tests/norm-rule/test.yaml

@@ -2,6 +2,6 @@
 
 == Chapter 1
 
-Before inline [#bad:duplicate]#inside inline# outside inline.
+Before inline [#duplicate:bad]#inside inline# outside inline.
 
-Same tag [#bad:duplicate]#duplicated inside# duplicated outside.
+Same tag [#duplicate:bad]#duplicated inside# duplicated outside.

tests/tags/duplicate.adoc

@@ -243,10 +243,14 @@ def check_allowed_types(kind, nr_name, name)
 end
 
 def fatal(msg)
-  puts "#{PN}: ERROR: #{msg}"
+  error(msg)
   exit(1)
 end
 
+def error(msg)
+  puts "#{PN}: ERROR: #{msg}"
+end
+
 def info(msg)
   puts "#{PN}: #{msg}"
 end
@@ -256,6 +260,7 @@ def usage(exit_status = 1)
   puts "  -h             Display this usage message"
   puts "  -j             Set output format to JSON (default)"
   puts "  -x             Set output format to XLSX"
+  puts "  -w             Warning instead of error if tags found without rules (Only use for debugging!)"
   puts "  -d filename    Normative rule definition filename (YAML format)"
   puts "  -t filename    Normative tag filename (JSON format)"
   puts
@@ -276,6 +281,7 @@ def parse_argv
   tag_fnames=[]
   output_fname=nil
   output_format="json"
+  warn_if_tags_no_rules = 0
 
   i = 0
   while i < ARGV.count
@@ -287,6 +293,8 @@ def parse_argv
       output_format = "json"
     when "-x"
       output_format = "xlsx"
+    when "-w"
+      warn_if_tags_no_rules = 1
     when "-d"
       if (ARGV.count-i) < 1
         info("Missing argument for -d option")
@@ -331,7 +339,7 @@ def parse_argv
     usage
   end
 
-  return [def_fnames, tag_fnames, output_fname, output_format]
+  return [def_fnames, tag_fnames, output_fname, output_format, warn_if_tags_no_rules]
 end
 
 # Load the contents of all normative rule tag files in JSON format.
@@ -492,7 +500,7 @@ def create_normative_rules_hash(defs, tags)
 
 # Fatal error if any normative rule references a non-existant tag
 # Warning if there are tags that no rule references.
-def validate_defs_and_tags(defs, tags)
+def validate_defs_and_tags(defs, tags, warn_if_tags_no_rules)
   fatal("Need NormativeRuleDefs for defs but passed a #{defs.class}") unless defs.is_a?(NormativeRuleDefs)
   fatal("Need NormativeTags for tags but was passed a #{tags.class}") unless tags.is_a?(NormativeTags)
 
@@ -511,7 +519,7 @@ def validate_defs_and_tags(defs, tags)
 
         if tag.nil?
           missing_tag_cnt = missing_tag_cnt + 1
-          info("Normative rule #{d.name} defined in file #{d.def_filename} references non-existent tag #{tag_ref_name}")
+          error("Normative rule #{d.name} defined in file #{d.def_filename} references non-existent tag #{tag_ref_name}")
         else
           referenced_tags[tag.name] = 1 # Any non-nil value
         end
@@ -522,13 +530,30 @@ def validate_defs_and_tags(defs, tags)
   # Look for any unreferenced tags.
   tags.get_tags.each do |tag|
     if referenced_tags[tag.name].nil?
-      info("Tag #{tag.name} not referenced by any normative rule. Did you forget to define a normative rule?")
+      msg = "Tag #{tag.name} not referenced by any normative rule. Did you forget to define a normative rule?"
+      if warn_if_tags_no_rules == 1
+        info(msg)
+      else
+        error(msg)
+      end
       unref_cnt = unref_cnt + 1
     end
   end
 
-  fatal("#{missing_tag_cnt} reference#{missing_tag_cnt == 1 ? "" : "s"} to non-existing tags") if missing_tag_cnt > 0
-  info("#{unref_cnt} tag#{unref_cnt == 1 ? "" : "s"} have no normative rules referencing them") if unref_cnt > 0
+  if missing_tag_cnt > 0
+    error("#{missing_tag_cnt} reference#{missing_tag_cnt == 1 ? "" : "s"} to non-existing tags")
+  end
+
+  if unref_cnt > 0
+    msg = "#{unref_cnt} tag#{unref_cnt == 1 ? "" : "s"} have no normative rules referencing them"
+    if warn_if_tags_no_rules == 1
+      info(msg)
+    else
+      error(msg)
+    end
+  end
+
+  fatal("Exiting due to errors") if ((missing_tag_cnt > 0) || ((unref_cnt > 0) && (warn_if_tags_no_rules == 0)))
 end
 
 # Store normative rules in JSON output file
@@ -568,7 +593,7 @@ def output_xlsx(filename, defs, tags)
   info("Storing #{defs.norm_rule_defs.count} normative rules into file #{filename}")
 
   # Create a new Excel workbook
-  info("Creating Excel workboook #{filename}")
+  info("Creating Excel workbook #{filename}")
   workbook = WriteXLSX.new(filename)
 
   # Add a worksheet
@@ -593,8 +618,8 @@ def output_xlsx(filename, defs, tags)
   # Add normative rules in rows. One row for each tag if multiple tags.
   row_num = 1
   defs.norm_rule_defs.each do |d|
-    worksheet.write(row_num, 0, d.chapter_name)
-    worksheet.write(row_num, 1, d.name, wrap_format)
+    worksheet.write_string(row_num, 0, d.chapter_name)
+    worksheet.write_string(row_num, 1, d.name, wrap_format)
 
     rule_defs = []
     rule_def_sources = []
@@ -619,10 +644,10 @@ def output_xlsx(filename, defs, tags)
     end
     rule_def_sources.append('[' + tag_sources.join(', ') + ']') unless tag_sources.empty?
 
-    worksheet.write(row_num, 2, rule_defs.join("\n"), wrap_format) unless rule_defs.empty?
-    worksheet.write(row_num, 3, rule_def_sources.join(", "), wrap_format) unless rule_def_sources.empty?
-    worksheet.write(row_num, 4, d.kind) unless d.kind.nil?
-    worksheet.write(row_num, 5, d.instances.join(', ')) unless d.instances.empty?
+    worksheet.write_string(row_num, 2, rule_defs.join("\n"), wrap_format) unless rule_defs.empty?
+    worksheet.write_string(row_num, 3, rule_def_sources.join(", "), wrap_format) unless rule_def_sources.empty?
+    worksheet.write_string(row_num, 4, d.kind) unless d.kind.nil?
+    worksheet.write_string(row_num, 5, d.instances.join(', ')) unless d.instances.empty?
 
     row_num += 1
   end
@@ -647,7 +672,7 @@ def output_xlsx(filename, defs, tags)
 
 info("Passed command-line: #{ARGV.join(' ')}")
 
-def_fnames, tag_fnames, output_fname, output_format = parse_argv()
+def_fnames, tag_fnames, output_fname, output_format, warn_if_tags_no_rules = parse_argv()
 
 info("Normative rule definition filenames = #{def_fnames}")
 info("Normative tag filenames = #{tag_fnames}")
@@ -656,7 +681,7 @@ def output_xlsx(filename, defs, tags)
 
 defs = load_definitions(def_fnames)
 tags = load_tags(tag_fnames)
-validate_defs_and_tags(defs, tags)
+validate_defs_and_tags(defs, tags, warn_if_tags_no_rules)
 
 case output_format
 when "json"