Empty test suite when backtic'ed object or def name contains no whitespace

[GoogleCodeExporter]

What steps will reproduce the problem?

class SampleSpec extends Spec with MustMatchers {
  object `foo` {
    def `baz` {
      true must be === true
    }
  }
}

What is the expected output? What do you see instead?

There should be one test in the spec, but there are none

Intellij: Empty test suite.

SBT:
[info] SampleSpec:
[info] Passed: : Total 0, Failed 0, Errors 0, Passed 0, Skipped 0

What version of the product are you using? On what operating system?
ScalaTest 2.0.M5b. Windows 7.

Please provide any additional information below.

(1) The following also does not work (because although the 'object' name 
contains whitespace the 'def' does not.)

class SampleSpec extends Spec with MustMatchers {
  object `foo baz` {
    def `baz` {
      true must be === true
    }
  }
}

gives:

Intellij: Empty test suite.

SBT:
[info] SampleSpec:
[info] foo baz
[info] Passed: : Total 0, Failed 0, Errors 0, Passed 0, Skipped 0


(2) The following does work (whitespace in both)

class SampleSpec extends Spec with MustMatchers {
  object `foo baz` {
    def `baz wibble` {
      true must be === true
    }
  }
}

gives:

Intellij:  1 Test passed

SBT:
[info] SampleSpec:
[info] foo baz
[info] - baz wibble
[info] Passed: : Total 1, Failed 0, Errors 0, Passed 1, Skipped 0

Original issue reported on code.google.com by [email protected] on 1 Jul 2013 at 3:49

[GoogleCodeExporter]

This documented in the Spec scaladoc but it would be nice to not have to 
remember to include a space.

Original comment by channing.walton on 1 Jul 2013 at 4:07

[GoogleCodeExporter]

Hi Channing,

The only way we could avoid the space requirement is to have people start with 
something like "test" or "should", or use an annotation like @Test, or some 
other creative approach. In some way we need to be able to tell the difference 
between a regular method and a test method, and a regular nested object and a 
"scope" one.

Original comment by [email protected] on 1 Jul 2013 at 4:29

[GoogleCodeExporter]

Hi Paul and Channing,

Actually let me add one bit of info. The backticks in Scala just transform 
non-identifier characters into identifier characters. After that's done, you 
can't really tell if back ticks were used or not. So what we do in Spec is look 
for a $u0020 in the name.  Here's an example:

scala> def `hi there` = "hi"
hi$u0020there: String

Here's what it looks like if you put backticks around something with all 
identifier-characters:

scala> def `howdy` = "hi"
howdy: String

Looks exactly the same as no identifier chars:

scala> def howdy = "hi"
howdy: String

So what we ask folks to do in Spec is put a space at the end in that case, 
because that gets a $u0020 in there but stays invisible in the Spec output:

scala> def `howdy ` = "hi"
howdy$u0020: String

All the "test method" frameworks have required you do something to identify a 
method as an actual test method. JUnit3 made you start each test method name 
with "test". JUnit4 and TestNG make you annotated it with @Test. ScalaTest's 
Spec trait makes you stick a $uu20 somewhere in the name.

Bill

Original comment by [email protected] on 1 Jul 2013 at 4:39

[GoogleCodeExporter]

Yes that makes sense. To be honest I don't think I would have a test named with 
a single word anyway so it's probably not an issue really. 

Original comment by channing.walton on 1 Jul 2013 at 6:13

[GoogleCodeExporter]

Well, it is unfortunately somewhat error prone because sometimes people write 
scopes with one word, and every now and then someone will likely make a test 
with just one word, and they'll not realize the space is the marker. And 
that'll waste some time until they figure it out.

I did try using annotations. @Test is concise and everyone is used to it, but 
it felt heavyweight and didn't fit very well with BDD. I tried @Specify, but 
again it just felt very heavyweight. The upside to that, though, it is makes it 
more obvious what the marker is, and I think less likely people will forget it.

So using spaces as the marker definitely involved a design tradeoff, but I 
think it was the best choice among the other options I could think of.

Original comment by [email protected] on 1 Jul 2013 at 8:24

[GoogleCodeExporter]

Hi All,

That makes perfect sense, my apologies I just started using it without reading 
the scaladocs. I was just surprised when some of my tests didnt run. I suspect 
I'm not the first of the last...

I'm migrating a load of tests from specs2 and was just blindly converting 
without thinking, some of those specs have names in camel case and therefore 
have no whitespace. 

I agree that writing a 'def' without whitespace is probably wrong/unlikely, but 
I can definitely see cases where it would be valid for the 'object' name to not 
have whitespace in it. 

A few thoughts:
(1) if an empty test suite was always a failure rather than a success, we would 
have noticed much earlier that our specs were being ignored. Perhaps it could 
fail with a helpful message like 'No tests found - are you sure your objects 
and defs contain a space character?'

(2) if all test defs had whitespace, would it possible to use that as the only 
critera and avoid the whitespace check for objects?

(3) with regard to understanding which methods are real tests and which are 
regular methods, would it be possible/feasible to say that all public methods 
are tests and that all regular methods should be private? 

Paul

Original comment by [email protected] on 2 Jul 2013 at 8:07

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Thanks, Paul, You made some excellent suggestions. I have been worried about 
the error prone-ness problem from the beginning, because I want ScalaTest to be 
easy to use *without* reading the documentation. I think the main one that can 
happen is a scope with no space, and your idea of either a) allowing that if 
the scope contains methods or nested objects with spaces in the names, or b) at 
least aborting the suite in that case, would be a big help. I'll think about 
whether a or b is better, but my initial feeling would be that a (i.e., your 
suggestion #2) is better. It would take slightly longer to do discovery this 
way, but should still be plenty fast enough.

As far as doing something with test classes with no tests or nested suites, 
your suggestion number 2, I think if we did the previous one, that would catch 
the case of an empty test suite when there's just one or more no-space scopes 
surrounding all tests. I could issue a warning, but I have the feeling that it 
would be unnecessary after the previous enhancement.

Lastly, I think it would be annoying to declare public methods as tests. People 
want to just make helper defs and it would be as non-obvious and error prone 
that those would need to be private as it is that test methods currently need a 
space character. toString would technically be a test method too! We could say 
this is true just inside a scope, but I think that would end up being annoying 
to folks too.

I think we'd want to change Spec's specification to say that object names with 
no spaces can be interpreted as scopes *so long as* it contains nested scopes 
or methods that have spaces in their names any level down. For example this 
should work:

object `Stack` {
  object `empty` {
    def `should be empty` {
    } 
  }
}

The "Stack" doesn't actually contain any directly nested objects ore methods 
with spaces in their names, but it does contain one no-space object "empty" 
that itself contains spaces. The one thing this would not catch is test method 
names themselves that had no spaces. So there's still some error-proneness 
there, but the biggest error-prone problem is fixed.

Original comment by [email protected] on 2 Jul 2013 at 12:54

[GoogleCodeExporter]

+1 to force every public method to be a scope/test or any other way of removing 
the posibility of false positives due to tests not being run. I don't think it 
is a good idea to have helper methods in the test class unless they are used 
only internally any way, and in that case they don't need to be public, do they?

(sorry to add a new post for a +1, there was not clear post to star to support 
only this bit)

Original comment by [email protected] on 14 Mar 2014 at 2:52