From 660a17cd438afda2c734b7dc396b8ede321fb79f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Cie=C5=9Blak?= Date: Mon, 21 Sep 2015 11:11:31 +0000 Subject: [PATCH 1/7] Allow testing for exitcode and stderr output If files "error" and "status" are existing in the test directory, do expect errors and test against them. Also create error and status files on --nuke. An error will be reported in this case anyway. Fixes: https://github.com/sass/sass-spec/issues/136 Conflicts: lib/sass_spec/runner.rb --- lib/sass_spec/runner.rb | 20 +++++++++++++++---- lib/sass_spec/test.rb | 39 +++++++++++++++++++++++++++++--------- lib/sass_spec/test_case.rb | 34 ++++++++++++++++++++++++++++++++- 3 files changed, 79 insertions(+), 14 deletions(-) diff --git a/lib/sass_spec/runner.rb b/lib/sass_spec/runner.rb index 436bb025bc..c4b01a60be 100644 --- a/lib/sass_spec/runner.rb +++ b/lib/sass_spec/runner.rb @@ -39,14 +39,26 @@ def _get_cases glob = File.join(@options[:spec_directory], "**", input_file) Dir.glob(glob) do |filename| input = Pathname.new(filename) + folder = File.dirname(filename) + expected_stderr_file_path = File.join(folder, "error") + expected_status_file_path = File.join(folder, "status") @options[:output_styles].each do |output_style| - folder = File.dirname(filename) output_file_name = @options["#{output_style}_output_file".to_sym] - expected_file_path = File.join(folder, output_file_name + ".css") + expected_stdout_file_path = File.join(folder, output_file_name + ".css") clean_file_name = File.join(folder, output_file_name + ".clean") - if File.file?(expected_file_path) && !File.file?(expected_file_path.sub(/\.css$/, ".skip")) && filename.include?(@options[:filter]) + if ( File.file?(expected_stdout_file_path) || + File.file?(expected_stderr_file_path) || + File.file?(expected_status_file_path) || + @options[:nuke] + ) && + !File.file?(expected_stdout_file_path.sub(/\.css$/, ".skip")) && + filename.include?(@options[:filter]) clean = File.file?(clean_file_name) - cases.push SassSpec::TestCase.new(input.realpath(), expected_file_path, output_style, clean, @options) + cases.push SassSpec::TestCase.new(input.realpath(), + expected_stdout_file_path, + expected_stderr_file_path, + expected_status_file_path, + output_style, clean, @options) end end end diff --git a/lib/sass_spec/test.rb b/lib/sass_spec/test.rb index 9dab45f7aa..bf0a1fc18b 100644 --- a/lib/sass_spec/test.rb +++ b/lib/sass_spec/test.rb @@ -6,11 +6,33 @@ def run_spec_test(test_case, options = {}) end assert File.exists?(test_case.input_path), "Input #{test_case.input_path} file does not exist" - assert File.exists?(test_case.expected_path), "Expected #{test_case.expected_path} file does not exist" output, clean_output, error, status = test_case.output - if status != 0 && !options[:unexpected_pass] + if options[:nuke] + if status != 0 + File.open(test_case.status_path, "w+") do |f| + f.write(status) + f.close + end + end + + if error.length > 0 + File.open(test_case.error_path, "w+") do |f| + f.write(error) + f.close + end + end + + File.open(test_case.expected_path, "w+") do |f| + f.write(output) + f.close + end + end + + assert File.exists?(test_case.expected_path), "Expected #{test_case.expected_path} file does not exist" + + if status != 0 && !options[:unexpected_pass] && (options[:nuke] || !test_case.should_fail?) msg = "Command `#{options[:engine_adapter]}` did not complete:\n\n#{error}" if options[:skip] @@ -26,17 +48,16 @@ def run_spec_test(test_case, options = {}) raise "#{test_case.input_path} passed a test we expected it to fail" end - if options[:nuke] - File.open(test_case.expected_path, "w+") do |f| - f.write(output) - f.close - end - end - if test_case.todo? && options[:unexpected_pass] assert test_case.expected != clean_output, "Marked as todo and passed" elsif !test_case.todo? || !options[:skip_todo] + if test_case.should_fail? + assert_equal test_case.expected_status, status, "Expected did not match status" + end assert_equal test_case.expected, clean_output, "Expected did not match output" + if test_case.verify_stderr? + assert_equal test_case.expected_error, error, "Expected did not match error" + end end end diff --git a/lib/sass_spec/test_case.rb b/lib/sass_spec/test_case.rb index d265a44b74..daf9b5a6b0 100644 --- a/lib/sass_spec/test_case.rb +++ b/lib/sass_spec/test_case.rb @@ -1,8 +1,10 @@ # This represents a specific test case. class SassSpec::TestCase - def initialize(input_scss, expected_css, style, clean, options = {}) + def initialize(input_scss, expected_css, error_file, status_file, style, clean, options = {}) @input_path = input_scss @expected_path = expected_css + @error_path = error_file + @status_path = status_file @output_style = style @clean_test = clean @options = options @@ -28,6 +30,22 @@ def expected_path @expected_path end + def error_path + @error_path + end + + def verify_stderr? + File.file?(@error_path) + end + + def status_path + @status_path + end + + def should_fail? + File.file?(@status_path) + end + def todo? @input_path.to_s.include? "todo" end @@ -36,7 +54,9 @@ def output if @output return @output end + stdout, stderr, status = engine.compile(@input_path, @output_style) + if @clean_test cleaned = _clean_output(stdout) else @@ -54,6 +74,18 @@ def expected end end + def expected_error + @expected_error = File.read(@error_path, :encoding => "utf-8") + end + + def expected_status + if should_fail? + @expected_status = File.read(@status_path, :encoding => "utf-8").to_i + else + @expected_status = 0 + end + end + def engine @options[:engine_adapter] end From 235738d4f1e274e71461d1039f9f4da9c2193697 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Cie=C5=9Blak?= Date: Mon, 21 Sep 2015 11:19:12 +0000 Subject: [PATCH 2/7] --skip is now no-op Convert --skip/-s to no-op, as this is now the default behavior. Rationale: As we now expect errors to be thrown by the spec scripts it makes no sense to stop whenever a first error is encountered. Unfortunately we cannot print deprecation warning in order not to break formats like tap or mess with stderr (Windows PowerShell thinks for example that if there is anything on stderr, the command failed). --- lib/sass_spec/cli.rb | 2 +- lib/sass_spec/test.rb | 7 +------ 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/lib/sass_spec/cli.rb b/lib/sass_spec/cli.rb index ed391d49a1..d861e9f7bb 100644 --- a/lib/sass_spec/cli.rb +++ b/lib/sass_spec/cli.rb @@ -69,7 +69,7 @@ def self.parse end opts.on("-s", "--skip", "Skip tests that fail to exit successfully") do - options[:skip] = true + # Note: --skip is no longer necessary as this is now the default behavior, since we test for errors end opts.on("--nuke", "Write a new expected_output for every test from whichever engine we are using") do diff --git a/lib/sass_spec/test.rb b/lib/sass_spec/test.rb index bf0a1fc18b..538f97d8f3 100644 --- a/lib/sass_spec/test.rb +++ b/lib/sass_spec/test.rb @@ -35,12 +35,7 @@ def run_spec_test(test_case, options = {}) if status != 0 && !options[:unexpected_pass] && (options[:nuke] || !test_case.should_fail?) msg = "Command `#{options[:engine_adapter]}` did not complete:\n\n#{error}" - if options[:skip] - raise msg - end - - puts msg - exit 4 + raise msg end From 6d125a043fa79220f3090fe3cf7cf85a3b2db023 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Cie=C5=9Blak?= Date: Sat, 19 Sep 2015 01:35:05 +0000 Subject: [PATCH 3/7] Relax status value checking This is a workaround to reconcile Ruby Sass and sassc: Ruby Sass returns values like 65, sassc returns 1. --- lib/sass_spec/test.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/sass_spec/test.rb b/lib/sass_spec/test.rb index 538f97d8f3..254943d0f2 100644 --- a/lib/sass_spec/test.rb +++ b/lib/sass_spec/test.rb @@ -47,7 +47,8 @@ def run_spec_test(test_case, options = {}) assert test_case.expected != clean_output, "Marked as todo and passed" elsif !test_case.todo? || !options[:skip_todo] if test_case.should_fail? - assert_equal test_case.expected_status, status, "Expected did not match status" + # XXX Ruby returns 65 etc. SassC returns 1 + assert status > 0, "Expected did not match status" end assert_equal test_case.expected, clean_output, "Expected did not match output" if test_case.verify_stderr? From d74dbe2fd53a4d8673e612555d2ff3c3ca5349d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Cie=C5=9Blak?= Date: Sun, 20 Sep 2015 00:23:06 +0000 Subject: [PATCH 4/7] Relax error value checking We cannot reliably compare full stacktraces and other forms of elaborate error reporting, therefore just compare only the first line of the standard error output. This obviously will not work for cases like testing for successive @warn directives. --- lib/sass_spec/test.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/sass_spec/test.rb b/lib/sass_spec/test.rb index 254943d0f2..31491364f6 100644 --- a/lib/sass_spec/test.rb +++ b/lib/sass_spec/test.rb @@ -52,7 +52,10 @@ def run_spec_test(test_case, options = {}) end assert_equal test_case.expected, clean_output, "Expected did not match output" if test_case.verify_stderr? - assert_equal test_case.expected_error, error, "Expected did not match error" + # Compare only first line of error output (we can't compare stacktraces etc.) + error_msg = error.each_line.next.rstrip + expected_error_msg = test_case.expected_error.each_line.next.rstrip + assert_equal expected_error_msg, error_msg, "Expected did not match error" end end end From e84b2798c93c21da3c12e09e5d71eeb02fb85984 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Cie=C5=9Blak?= Date: Sun, 20 Sep 2015 01:56:57 +0000 Subject: [PATCH 5/7] Capture error output reliably We cannot run tests in parallel when redirecting $stderr into a single buffer Fixes: https://github.com/sass/sass-spec/issues/499 --- lib/sass_spec/engine_adapter.rb | 3 ++- lib/sass_spec/test.rb | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/sass_spec/engine_adapter.rb b/lib/sass_spec/engine_adapter.rb index 8305c1c588..8776e10627 100644 --- a/lib/sass_spec/engine_adapter.rb +++ b/lib/sass_spec/engine_adapter.rb @@ -68,10 +68,11 @@ def compile(sass_filename, style) require 'sass' begin captured_stderr = StringIO.new + # Does not work as expected when tests are run in parallel real_stderr, $stderr = $stderr, captured_stderr begin css_output = Sass.compile_file(sass_filename.to_s, :style => style.to_sym) - [css_output, captured_stderr.to_s, 0] + [css_output, captured_stderr.string, 0] rescue Sass::SyntaxError => e ["", "Error: " + e.message.to_s, 1] rescue => e diff --git a/lib/sass_spec/test.rb b/lib/sass_spec/test.rb index 31491364f6..593db4444a 100644 --- a/lib/sass_spec/test.rb +++ b/lib/sass_spec/test.rb @@ -63,7 +63,6 @@ def run_spec_test(test_case, options = {}) # Holder to put and run test cases class SassSpec::Test < Minitest::Test - parallelize_me! def self.create_tests(test_cases, options = {}) test_cases[0..options[:limit]].each do |test_case| define_method('test__' << test_case.output_style + "_" + test_case.name) do From f9868be9f31e4292fd9e81085a72a811f59632df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Cie=C5=9Blak?= Date: Mon, 21 Sep 2015 11:29:16 +0000 Subject: [PATCH 6/7] Add test for duplicate map keys Still marked as "todo" due to libsass message mismatch. Test for https://github.com/sass/libsass/issues/628 Fixes https://github.com/sass/sass-spec/pull/133 --- spec/maps/duplicate-keys-todo/error | 3 +++ spec/maps/duplicate-keys-todo/expected.compact.css | 0 spec/maps/duplicate-keys-todo/expected.compressed.css | 0 spec/maps/duplicate-keys-todo/expected.expanded.css | 0 spec/maps/duplicate-keys-todo/expected_output.css | 0 spec/maps/duplicate-keys-todo/input.scss | 9 +++++++++ spec/maps/duplicate-keys-todo/status | 1 + 7 files changed, 13 insertions(+) create mode 100644 spec/maps/duplicate-keys-todo/error create mode 100644 spec/maps/duplicate-keys-todo/expected.compact.css create mode 100644 spec/maps/duplicate-keys-todo/expected.compressed.css create mode 100644 spec/maps/duplicate-keys-todo/expected.expanded.css create mode 100644 spec/maps/duplicate-keys-todo/expected_output.css create mode 100644 spec/maps/duplicate-keys-todo/input.scss create mode 100644 spec/maps/duplicate-keys-todo/status diff --git a/spec/maps/duplicate-keys-todo/error b/spec/maps/duplicate-keys-todo/error new file mode 100644 index 0000000000..a848af4dd7 --- /dev/null +++ b/spec/maps/duplicate-keys-todo/error @@ -0,0 +1,3 @@ +Error: Duplicate key "eta" in map (eta: 5, eta: 6). + on line 5 of /home/saper/sw/libsass/sass-spec/spec/maps/duplicate-keys/input.scss + Use --trace for backtrace. diff --git a/spec/maps/duplicate-keys-todo/expected.compact.css b/spec/maps/duplicate-keys-todo/expected.compact.css new file mode 100644 index 0000000000..e69de29bb2 diff --git a/spec/maps/duplicate-keys-todo/expected.compressed.css b/spec/maps/duplicate-keys-todo/expected.compressed.css new file mode 100644 index 0000000000..e69de29bb2 diff --git a/spec/maps/duplicate-keys-todo/expected.expanded.css b/spec/maps/duplicate-keys-todo/expected.expanded.css new file mode 100644 index 0000000000..e69de29bb2 diff --git a/spec/maps/duplicate-keys-todo/expected_output.css b/spec/maps/duplicate-keys-todo/expected_output.css new file mode 100644 index 0000000000..e69de29bb2 diff --git a/spec/maps/duplicate-keys-todo/input.scss b/spec/maps/duplicate-keys-todo/input.scss new file mode 100644 index 0000000000..03ab71c028 --- /dev/null +++ b/spec/maps/duplicate-keys-todo/input.scss @@ -0,0 +1,9 @@ +$map: ( + alpha: 1, + beta: 2, + gamma: 3, + delta: ( + eta: 5, + eta: 6, + ), +); diff --git a/spec/maps/duplicate-keys-todo/status b/spec/maps/duplicate-keys-todo/status new file mode 100644 index 0000000000..b44fe09a7a --- /dev/null +++ b/spec/maps/duplicate-keys-todo/status @@ -0,0 +1 @@ +65 \ No newline at end of file From 0d6ad1ab0dc66369e0130bc669f041120361afb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Cie=C5=9Blak?= Date: Sat, 19 Sep 2015 00:42:06 +0000 Subject: [PATCH 7/7] Test @error and @warn Please note that @warn test has both an error output and normal output, and Sass compiler does not report failure (no "status" file). --- spec/misc/error-directive/error | 3 +++ spec/misc/error-directive/expected.compact.css | 0 spec/misc/error-directive/expected.compressed.css | 0 spec/misc/error-directive/expected.expanded.css | 0 spec/misc/error-directive/expected_output.css | 0 spec/misc/error-directive/input.scss | 1 + spec/misc/error-directive/status | 1 + spec/misc/warn-directive/error | 3 +++ spec/misc/warn-directive/expected.compact.css | 1 + spec/misc/warn-directive/expected.compressed.css | 1 + spec/misc/warn-directive/expected.expanded.css | 3 +++ spec/misc/warn-directive/expected_output.css | 2 ++ spec/misc/warn-directive/input.scss | 2 ++ 13 files changed, 17 insertions(+) create mode 100644 spec/misc/error-directive/error create mode 100644 spec/misc/error-directive/expected.compact.css create mode 100644 spec/misc/error-directive/expected.compressed.css create mode 100644 spec/misc/error-directive/expected.expanded.css create mode 100644 spec/misc/error-directive/expected_output.css create mode 100644 spec/misc/error-directive/input.scss create mode 100644 spec/misc/error-directive/status create mode 100644 spec/misc/warn-directive/error create mode 100644 spec/misc/warn-directive/expected.compact.css create mode 100644 spec/misc/warn-directive/expected.compressed.css create mode 100644 spec/misc/warn-directive/expected.expanded.css create mode 100644 spec/misc/warn-directive/expected_output.css create mode 100644 spec/misc/warn-directive/input.scss diff --git a/spec/misc/error-directive/error b/spec/misc/error-directive/error new file mode 100644 index 0000000000..dbc386f141 --- /dev/null +++ b/spec/misc/error-directive/error @@ -0,0 +1,3 @@ +Error: Buckle your seatbelt Dorothy, 'cause Kansas is going bye-bye + on line 1 of /home/saper/sw/libsass/sass-spec/spec/misc/error-directive/input.scss + Use --trace for backtrace. diff --git a/spec/misc/error-directive/expected.compact.css b/spec/misc/error-directive/expected.compact.css new file mode 100644 index 0000000000..e69de29bb2 diff --git a/spec/misc/error-directive/expected.compressed.css b/spec/misc/error-directive/expected.compressed.css new file mode 100644 index 0000000000..e69de29bb2 diff --git a/spec/misc/error-directive/expected.expanded.css b/spec/misc/error-directive/expected.expanded.css new file mode 100644 index 0000000000..e69de29bb2 diff --git a/spec/misc/error-directive/expected_output.css b/spec/misc/error-directive/expected_output.css new file mode 100644 index 0000000000..e69de29bb2 diff --git a/spec/misc/error-directive/input.scss b/spec/misc/error-directive/input.scss new file mode 100644 index 0000000000..b9c971fd6c --- /dev/null +++ b/spec/misc/error-directive/input.scss @@ -0,0 +1 @@ +@error "Buckle your seatbelt Dorothy, 'cause Kansas is going bye-bye" diff --git a/spec/misc/error-directive/status b/spec/misc/error-directive/status new file mode 100644 index 0000000000..b44fe09a7a --- /dev/null +++ b/spec/misc/error-directive/status @@ -0,0 +1 @@ +65 \ No newline at end of file diff --git a/spec/misc/warn-directive/error b/spec/misc/warn-directive/error new file mode 100644 index 0000000000..278fac29f1 --- /dev/null +++ b/spec/misc/warn-directive/error @@ -0,0 +1,3 @@ +WARNING: Don't crash the ambulance, whatever you do + on line 2 of /home/saper/sw/libsass/sass-spec/spec/misc/warn-directive/input.scss + diff --git a/spec/misc/warn-directive/expected.compact.css b/spec/misc/warn-directive/expected.compact.css new file mode 100644 index 0000000000..08762eb97e --- /dev/null +++ b/spec/misc/warn-directive/expected.compact.css @@ -0,0 +1 @@ +h1 { color: blue; } diff --git a/spec/misc/warn-directive/expected.compressed.css b/spec/misc/warn-directive/expected.compressed.css new file mode 100644 index 0000000000..b1c489872d --- /dev/null +++ b/spec/misc/warn-directive/expected.compressed.css @@ -0,0 +1 @@ +h1{color:blue} diff --git a/spec/misc/warn-directive/expected.expanded.css b/spec/misc/warn-directive/expected.expanded.css new file mode 100644 index 0000000000..ccf22a056c --- /dev/null +++ b/spec/misc/warn-directive/expected.expanded.css @@ -0,0 +1,3 @@ +h1 { + color: blue; +} diff --git a/spec/misc/warn-directive/expected_output.css b/spec/misc/warn-directive/expected_output.css new file mode 100644 index 0000000000..30b5b22a57 --- /dev/null +++ b/spec/misc/warn-directive/expected_output.css @@ -0,0 +1,2 @@ +h1 { + color: blue; } diff --git a/spec/misc/warn-directive/input.scss b/spec/misc/warn-directive/input.scss new file mode 100644 index 0000000000..798462fbdd --- /dev/null +++ b/spec/misc/warn-directive/input.scss @@ -0,0 +1,2 @@ +h1 { color: blue; } +@warn "Don't crash the ambulance, whatever you do"