Merge pull request #1406 from sass/merge-into-use

Merge master into feature.use

README.md

@@ -197,7 +197,7 @@ eventually be removed.
 ```yaml
 ---
 :todo:
-- libsass # sass/libsass#2827
+- sass/libsass#2827
 ```
 
 This option indicates implementations that should add support for a spec, but
@@ -206,9 +206,11 @@ marked as `:todo` for that implementation are skipped by default. This ensures t
 the build remains green while clearly marking which specs are expected to pass
 eventually.
 
-In general, when marking a spec as `:todo` for an implementation, it's a good
-idea to include a comment referencing the GitHub issue for that implementation
-that will fix the spec. If no issue exists yet, please file one.
+Implementations can be (and should be) specified as shorthand GitHub issue
+references rather than plain names. This makes it easy to track whether the
+implementation has fixed the issue, and to see which specs correspond to which
+issue. When marking an issue as `:todo` for an implementation, please either
+find an existing issue to reference or file a new one.
 
 If the `--run-todo` flag is passed to `sass-spec.rb`, specs marked as `:todo`
 for the current implementation will be run, and their failures will be reported.
@@ -225,7 +227,7 @@ these specs.
 ```yaml
 ---
 :warning_todo:
-- libsass # sass/libsass#2834
+- sass/libsass#2834
 ```
 
 This option works like [`:todo`](#todo), except instead of skipping the entire

STYLE_GUIDE.md

@@ -754,7 +754,7 @@ be named "other" (`_other.scss`, `_other.sass`, etc).
 <===> slash_slash_string/options.yml
 ---
 :todo:
-- libsass # sass/libsass#2840
+- sass/libsass#2840
 
 <===> slash_slash_string/input.scss
 a {b: 1 / 2 / foo()}
@@ -784,10 +784,10 @@ a {
 
 </details>
 
-When [marking a spec as `:todo`][], follow it with a comment that refers to an
-issue in the implementation in question tracking the feature the spec tests. If
-there isn't an issue yet, create one. Make sure to also link that issue back to
-the specs for it!
+When [marking a spec as `:todo`][], use a reference to an issue in the
+implementation in question tracking the feature the spec tests. If there isn't
+an issue yet, create one. Make sure to also link that issue back to the specs
+for it!
 
 [marking a spec as `:todo`]: README.md#todo
 
@@ -996,7 +996,7 @@ directory. It just serves to visually separate specs from one another.
 <===> top_level/options.yml
 ---
 :warning_todo:
-- libsass # sass/libsass#2834
+- sass/libsass#2834
 :ignore_for:
 - ruby-sass
 
@@ -1041,7 +1041,7 @@ declare new variables. Consider adding `$var: null` at the top level.
 <===> top_level/options.yml
 ---
 :warning_todo:
-- libsass # sass/libsass#2834
+- sass/libsass#2834
 :ignore_for:
 - ruby-sass
 

lib/sass_spec/directory.rb

@@ -176,6 +176,15 @@ def with_real_files
     FileUtils.rm_rf(outermost_new_dir) if outermost_new_dir
   end
 
+  # Returns an HRX representation of this directory.
+  def to_hrx
+    archive = HRX::Archive.new
+    glob("**/*").each do |path|
+      archive << HRX::File.new("#{self.path}/#{path}", read(path))
+    end
+    archive.to_hrx
+  end
+
   def inspect
     "#<SassSpec::Directory:#{@path}>"
   end

lib/sass_spec/test.rb

@@ -128,28 +128,9 @@ def handle_conflicting_files!
     choice = interact(:conflicting_files, :fail) do |i|
       i.prompt "Test has both error and success outputs."
 
-      show_input_choice(i)
+      show_test_case_choice(i)
       show_output_choice(i)
 
-      if output_file_exists
-        i.choice('eo', 'Show expected output.') do
-          display_text_block(@test_case.read("output.css", impl: impl))
-          i.restart!
-        end
-      end
-
-      if warning_file_exists
-        i.choice('ew', 'Show expected warning.') do
-          display_text_block(@test_case.read("warning", impl: impl))
-          i.restart!
-        end
-      end
-
-      i.choice('ee', 'Show expected error.') do
-        display_text_block(@test_case.read("error", impl: :auto))
-        i.restart!
-      end
-
       delete_choice(i)
 
       i.choice('S', 'Keep the success output.') do
@@ -179,7 +160,7 @@ def handle_missing_output!
     choice = interact(:missing_output, :fail) do |i|
       i.prompt "Expected output file does not exist."
 
-      show_input_choice(i)
+      show_test_case_choice(i)
       show_output_choice(i)
       delete_choice(i)
       update_output_choice(i)
@@ -202,7 +183,7 @@ def handle_unexpected_error!
     choice = interact(:unexpected_error, :fail) do |i|
       i.prompt "An unexpected compiler error was encountered."
 
-      show_input_choice(i)
+      show_test_case_choice(i)
 
       i.choice('e', "Show me the error.") do
         display_text_block(@error)
@@ -234,12 +215,7 @@ def handle_unexpected_pass!
     choice = interact(:unexpected_pass, :fail) do |i|
       i.prompt "A failure was expected but it compiled instead."
 
-      show_input_choice(i)
-
-      i.choice('e', "Show me the expected error.") do
-        display_text_block(@test_case.expected_error)
-        i.restart!
-      end
+      show_test_case_choice(i)
 
       i.choice('o', "Show me the output.") do
         display_text_block(@output)
@@ -270,7 +246,7 @@ def handle_output_difference!
     interact(:output_difference, :fail) do |i|
       i.prompt "Output does not match expectation."
 
-      show_input_choice(i)
+      show_test_case_choice(i)
 
       i.choice('d', "show diff.") do
         require 'diffy'
@@ -297,14 +273,7 @@ def handle_unnecessary_todo!
     interact(:unnecessary_todo, :fail) do |i|
       i.prompt "Test is passing but marked as TODO."
 
-      show_input_choice(i)
-
-      if @test_case.should_fail?
-        i.choice('e', "Show me the expected error.") do
-          display_text_block(@test_case.expected_error)
-          i.restart!
-        end
-      end
+      show_test_case_choice(i)
 
       unless @output.empty?
         i.choice('o', "Show me the output.") do
@@ -367,26 +336,21 @@ def handle_stderr_difference!
     interact(type, :fail) do |i|
       i.prompt(message)
 
-      show_input_choice(i)
+      show_test_case_choice(i)
 
-      if error_msg.empty?
-        i.choice('e', "Show expected #{@test_case.should_fail? ? 'error' : 'warning'}.") do
-          display_text_block(@test.expected_error)
-          i.restart!
-        end
-      elsif expected_error_msg.empty?
+      unless error_msg.empty?
         i.choice('e', "Show #{@test_case.should_fail? ? 'error' : 'warning'}.") do
           display_text_block(error_msg)
           i.restart!
         end
-      else
-        i.choice('d', "Show diff.") do
-          require 'diffy'
-          display_text_block(
-            Diffy::Diff.new("Expected\n#{expected_error_msg}",
-                            "Actual\n#{error_msg}").to_s(:color))
-          i.restart!
-        end
+      end
+
+      i.choice('d', "Show diff.") do
+        require 'diffy'
+        display_text_block(
+          Diffy::Diff.new("Expected\n#{expected_error_msg}",
+                          "Actual\n#{error_msg}").to_s(:color))
+        i.restart!
       end
 
       update_output_choice(i)
@@ -413,9 +377,9 @@ def interact(prompt_id, default, &block)
     end
   end
 
-  def show_input_choice(i)
-    i.choice('i', "Show me the input.") do
-      display_text_block(@test_case.input)
+  def show_test_case_choice(i)
+    i.choice('t', "Show me the test case.") do
+      display_text_block(@test_case.dir.to_hrx)
       i.restart!
     end
   end

lib/sass_spec/test_case_metadata.rb

@@ -48,7 +48,14 @@ def self.merge_options(existing_opts, new_opts)
     # Parses metadata for the test case at the given SassSpec::Directory.
     def initialize(test_case_dir)
       @name = test_case_dir.to_s
-      @options = resolve_options(test_case_dir).freeze
+      @options = resolve_options(test_case_dir)
+    end
+
+    def resolve_options(dir)
+      self.class.cache[dir.path] ||= _resolve_options(dir).tap do |options|
+        _normalize_todos(options, :todo)
+        _normalize_todos(options, :warning_todo)
+      end.freeze
     end
 
     def _resolve_options(dir)
@@ -72,8 +79,14 @@ def _resolve_options(dir)
       rv
     end
 
-    def resolve_options(dir)
-      self.class.cache[dir.path] ||= _resolve_options(dir).freeze
+    # Normalize a list of TODOs to convert GitHub issue references to
+    # implementation names.
+    def _normalize_todos(options, field)
+      if options.include?(field)
+        options[field] = options[field]
+          .map {|name| name =~ %r{^sass/(.*)#} ? $1 : name}
+          .to_set
+      end
     end
 
     def todo?(impl)