search

jejacks0n / teaspoon

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

Extended runner.js to provide further error details #469

Closed sebastianzillessen closed 5 years ago

sebastianzillessen commented 8 years ago

If there has been an error on the teaspoon webpage (e.g. if the Spec helper tries to require a file/folder which does not exist for some reason as an empty folder that has been removed during a CI process) which will result in a 'success' return code, but does not render the Teaspoon test page, the error description was - especially while debugging on a remote CI - not really helpful.

This commit provides additional error descriptions in this case by rendering the plain_text of the returned website into the console output, it does not modify the functionality.

jejacks0n commented 8 years ago

I'll keep this around, but it probably won't be merged as is. So initially I think Teaspoon handled these things well, by telling Rails to convert the rendered output to a javascript exception, but that seems to not work anymore. I'd rather handle this at the suite_controller level and handle exceptions by rendering that exception as a javascript_tag "throw [exception]" sort of statement. That's probably a bit more elegant and robust. But thanks for bringing it to our attention -- it definitely needs some love there.

Worth noting, some gems, like better_errors can make this stuff not work. Are you using anything like that?

sebastianzillessen commented 8 years ago

Well, the thing is, it took me almost 3 hours to find the problem in my test suite, because teaspoon just tells u, it could not connect to the url ...:xx/teaspoon/suite if there exists any error on the page. The problem for me was, that I used a require_tree in my spec_helper, which was (as it was empty) ignored by the CI while cloning the repository and therefore my specs failed.

So I think it could safe other people time if they are just using the console or a CI environment, by providing them better error explanations.

And as it is only a problem in the Problem in the phantomjs/runner I think this is the right place for it.

Yes, we are indeed using better_errors, therefore I thought it would be more reliable to check it here as well, as the approach I took tackles the actual problem, namely that window.Teaspoon is not present in the page returned to phantomjs.

So to sum it up: I think we should include it, so that everyone sees error messages, if something goes wrong with the website while the tests are running.

jejacks0n commented 8 years ago

That's fair. I understand what you're trying to solve. There's likely a better way to do this, and I'd like to investigate that before considering your solution. Also, there aren't any specs here and it seems to have broken the build, but instead of saying that, I figured it might be worth looking into alternate approaches to help you resolve the problem without asking you for additional work.

I'm sorry Teaspoon fell down on you here, but it's really about how better_errors hooks in (or doesn't really hook in, but tends to stomp all over) how rails handles exceptions normally. That seems like a better thing to look into from my perspective.

sebastianzillessen commented 8 years ago

Alright, always glad to help. Let me just know, how you fixed it. :+1: