Fix error reporting regression

[mgreter]

Addresses #1506

[am11]

With:

div {
  height: 10px;

I am still getting:

Error: Invalid CSS after "": expected "}", was ""

With Ruby Sass, it produces:

Invalid CSS after "  height: 10px;": expected "}", was ""

(tested both with LF and CRLF on win)

[mgreter]

Hopefully this time ... and we urgently need some way to have these unit tested, otherwise it's nearly impossible to refactor this kind of thing!

[am11]

Thanks! This seems to be fixed with 8861bc1.

On a slightly related note, with ruby-sass the exception message reads as follow: ```bash vagrant@trusty64:~$ ruby -v ruby 2.2.3p173 (2015-08-18 revision 51636) [x86_64-linux-gnu] vagrant@trusty64:~$ sass -v Sass 3.4.18 (Selective Steve) vagrant@trusty64:~$ cat blah.scss div { height: 10px; vagrant@trusty64:~$ sass blah.scss Error: Invalid CSS after " height: 10px;": expected "}", was "" on line 3 of blah.scss Use --trace for backtrace. ``` without `...` part. Is it something which will be changed in libsass at some point?

Edit: was testing against wrong branch, there is no ellipsis mark (...): Fixed! :)

---

Regarding testing errors and other special / internal features of libsass, as per sass/sassc#90, in my humble opinion, we should invest effort only once and get rid of the Ruby dependency in favour of C++ testing framework. I will try to prototype salient aspects of testing in C++ using some slim framework (whichever is trending in other repos on GitHub) pretty 🔜 (will roll up my selves on coming weekend), in effort to make it a drop-in replacement for ruby test runner for sass-spec (like we have in node-sass). IMO, first three points to focus on would be:

• Running spec tests (keeping it TODOs and non-TODOs aware).
• Generating lcov compatible report for coverage.
• Using proper exception detection semantics, to test the error messages against the hardcoded ones.

On top of that, we can build infrastructure for source-map, memory management and performance tests etc. in a long run. :)
I realize that this is a lot of work and there is risk of things getting more delayed to develop the robust replacement, but IMVHO, there will be even more tech debt we would have to pay otherwise.

[drewwells]

I've been thinking the same. It must be very hard to develop on the API
without any unit tests.
On Wed, Sep 2, 2015 at 5:43 AM Adeel Mujahid [email protected]
wrote:

Thanks! This seems to be fixed with 8861bc1
8861bc1
.
On a slightly related note, with ruby-sass the exception message reads as
follow:

vagrant@trusty64:$ ruby -v
ruby 2.2.3p173 (2015-08-18 revision 51636) [x86_64-linux-gnu]
vagrant@trusty64:$ sass -v
Sass 3.4.18 (Selective Steve)
vagrant@trusty64:$ cat blah.scss
div {
height: 10px;
vagrant@trusty64:$ sass blah.scss
Error: Invalid CSS after " height: 10px;": expected "}", was ""
on line 3 of blah.scss
Use --trace for backtrace.

without ... part. Is it something which will be changed in libsass at

some point?

Regarding testing errors and other special / internal feature of libsass,
as per sass/sassc#90 sass/sassc#90, in my
humble opinion, we should invest effort only once and get rid of the Ruby
dependency in favour of C++ testing framework. I will try to prototype
salient aspects of testing in C++ using some slim framework (whichever is
trending in other repos on GitHub) pretty [image: 🔜](will roll up
my selves on coming weekend), in effort to make it a drop-in replacement
for ruby test runner for sass-spec (like we have in node-sass). IMO, first
three points focus on would be:

• Running spec tests (keeping it TODOs and non-TODOs aware).
• Generating lcov compatible report for coverage.
• Using proper exception detection semantics, to test the error
  messages against the hardcoded ones.

On top of that, we can build infrastructure for source-map, memory
management and performance tests etc. in a long run. :)
I realize that this is a lot of work and there is risk of things getting
more delayed to develop the robust replacement, but IMVHO, there will be
even more tech debt we would have do pay otherwise.

—
Reply to this email directly or view it on GitHub
#1512 (comment).

[saper]

To test an API you do integration tests. Unit testing requires decomposition of the code internally to test particular pieces. There are some stub tests in https:/sass/libsass/tree/202a4ea098c6f99ed70344135dbe3d4720e0c681/test which afaik are not connected to the CI. Getting them to work would be a good first step.

[xzyfer]

My 2c on this is that it makes sense to separate the c++ test runner from
the sass specs themselves. It's worth remembering that although we're the
primary user of sass spec, it exists to serve all Sass implementors. It
won't be long before Ruby Sass is using sass spec.
On 3 Sep 2015 02:12, "Marcin Cieślak" [email protected] wrote:

To test an API you do integration testy. Unit testing requires
decomposition of the code internally to test particular pieces. There are
some stub tests in
https:/sass/libsass/tree/202a4ea098c6f99ed70344135dbe3d4720e0c681/test
which afaik are not connected to the CI. Getting them to work would be a
good first step.

—
Reply to this email directly or view it on GitHub
#1512 (comment).

[am11]

Considering the intent is to keep Sass-spec repo neutral, independent of any runtime; all the consumers of Sass-spec will have the test tools in their own repos, written in the corresponding runtimes etc.

For example, node-sass which conceptually is a secondary consumer if we strictly go by the laws of food-chain, but practically sets an example of using Sass-spec as a fixtures-only submodule.