jejacks0n / teaspoon

Teaspoon: Javascript test runner for Rails. Use Selenium, BrowserStack, or PhantomJS.
1.43k stars 243 forks source link

SyntaxError in a build does not fail. #445

Closed hwhelchel closed 8 years ago

hwhelchel commented 8 years ago

I'm using 1.1.1 teaspoon and 2.3.4 teaspoon-jasmine

When I run the tests in browser I see that I am missing a parens. In CLI mode it displays that there is a syntax error but it doesn't fail the build.

CLI Mode screen shot 2016-01-06 at 9 52 29 am

Browser screen shot 2016-01-06 at 10 05 14 am

Thanks for your help!

hwhelchel commented 8 years ago

I'm also using angular js if that is helpful. Could be related but not sure.

jejacks0n commented 8 years ago

They don't, but we've thus far decided that it's not a failure directly.. I'd rather it be caught by the developer when the tests are run.. a Parse Error doesn't always mean that the javascript won't execute, so if the test coverage is "good enough", it should capture this, as well as it being visible in the ways you show.

I could be convinced otherwise, and every time it comes up I find myself being pretty much on the fence about it.

davetron5000 commented 8 years ago

Just lost about an hour to this. Here's an example that might make it more clear why this is undesirable behavior:

  1. Clone https://github.com/madblkman/Shine
  2. bundle install
  3. bundle exec rake teaspoon

You'll see 1 spec ran, 1 passed. But, if you look at spec/javascripts/customers_app/controllers/customer_search_controller_spec.js you'll see there are three more tests in there. These don't get mentioned or even show up.

Now, if you rails s and go to http://localhost:3000/teaspoon, you can see teaspoon/jasmine is picking it up, and shows there are 0 tests in that file, and all is good.

Only when you go to the console do you see syntax errors on line 78 and line 100. Fixing these and both the web and CLI show 4 tests run, 4 tests passed.

My CLI output doesn't even show an error, so there's no indication that something is wrong. And, it seems to me, something is terribly wrong if my spec files have syntax errors in them.

FWIW, in most other languages, a failure to compile/interpret the test code causes some sort of obvious failure.

maxhudson commented 7 years ago

@jejacks0n Agreed that it is developer's responsibility to catch syntax errors in actual code, but syntax errors in test code are very easy to miss when the suite passes. The only way to tell is the test count changing, but it still is green and obviously passes if you're using CI.

As a code-reviewer, this is even harder to catch because you're not keeping track of the number of tests that are supposed to pass, and you don't want to have to check the CI run every time it runs.