Refactor some spec infrastructure #1308
Conversation
For consistency, all derived attributes are now written as methods, rather than having some written as attr_readers and some as methods.
This adds a lot of complexity for no use-case that I can see. All specs always live in spec/.
TestCase now just represents the expectations of a given test case as written to the filesystem, and it's SassSpecRunner's job to actually run the implementation and verify its output.
This allows us to remove TestCase.result?, making TestCase entirely unmodifiable.
These were only used in one place, so it makes more sense to just manually put them there.
| @@ -42,24 +42,7 @@ def run | |||
| Minitest.reporter = Minitap::TapY | |||
| end | |||
|
|
|||
| result = Minitest.run(minioptions) | |||
|
|
|||
| if @options[:run_todo] | |||
There was a problem hiding this comment.
Think this is actually useful, since I think @xzyfer runs this to find what is passing in libsass, but I'll let him chime in
Can't recall if this just shadowed the probe_todo though
There was a problem hiding this comment.
It's a big pain in the ass to support, so it might be worth altering workflows for. You can still find passing TODO tests using --interactive --probe-todo.
| @@ -103,13 +103,6 @@ def self.parse | |||
| options[:only_output_styles] << style | |||
| end | |||
|
|
|||
| opts.on("-r SPEC_DIR", "--root SPEC_DIR", "Root directory for the specs. ", | |||
There was a problem hiding this comment.
Not sure spec directory is used by some of the libsass variants when skimming through https://github.com/search?p=1&q=org%3Asass+sass-spec&type=Code
There was a problem hiding this comment.
I see some stuff about configuring the location of the sass-spec gem itself, but nothing about using the --root flag.
There was a problem hiding this comment.
More generally: do any wrappers (other than Node Sass) use sass-spec in any kind of special way other than just invoking sass-spec with --command and --impl?
There was a problem hiding this comment.
Not possible since it would break the whole concept of cpan tests which mostly doesn't have ruby installed and also relies sometimes on GCC 4.4 : http://matrix.cpantesters.org/?dist=CSS-Sass%203.4.10
There was a problem hiding this comment.
Well, I guess consider this a heads-up that there will be some pretty substantial changes in test format coming soon.
There was a problem hiding this comment.
OK, not that I like to rewrite it for the third time ...
There was a problem hiding this comment.
That's why I was hoping it could re-use the Ruby runner 😞. Another solution would be to just not run the language specs, since it's just a wrapper and LibSass already tests its language implementation thoroughly.
There was a problem hiding this comment.
I understand, but the cpan test matrix actually did uncover a few quirks here and there already which weren't caught by libsass CI before and it's always a good feeling when it passed the full matrix.
No description provided.