TryGhost / Ghost

Independent technology for modern publishing, memberships, subscriptions and newsletters.
https://ghost.org
MIT License
47.24k stars 10.3k forks source link

Infra: Functional Tests are not reliable #1287

Closed ErisDS closed 10 years ago

ErisDS commented 10 years ago

See the output here: https://gist.github.com/ErisDS/aed9c041ef165e020c14

Test fail and then seem to go into an infinite loop.

gotdibbs commented 10 years ago

@ErisDS Is this an intermittent error? Have you seen it happen on travis? I'm assuming it's an issue with the current code in master?

Love how fragile casper tests are by the way :). Might be time to start shopping around (looking at other options like selenium).

halfdan commented 10 years ago

Happened to me on #1147. The underlying issue was that the content didn't load because of a change in node v0.11.x.

deedubs commented 10 years ago

The angular project is moving towards https://code.google.com/p/selenium/wiki/WebDriverJs they've built https://github.com/angular/protractor to make it easier to wrap it all up

gotdibbs commented 10 years ago

@deedubs Looks like a solid option.

I also know that @endangeredmassa has a project he's been working on that as I recall allows you to use mocha-style syntax on top of selenium, but execute your tests in a synchronous fashion. Since I think the majority of our issues are due to time outs, synchronous functional testing might be a dream come true for us. That's not to mention the fact that our unit tests are done in mocha as well.

All in all the easier it is to write and maintain the functional tests the better. We may need to evaluate several options before we can really resolve this.

ErisDS commented 10 years ago

There is also a library that allows for using mocha and should style syntax with casperjs.

I'm in two minds - one the one hand using webDriverJs would open up some more possibilities for us, it's more mature and works with things like Appium. On the other hand, I really love the casperjs project, and want to support it - plus it has such an epic name ;). Perhaps it would be worth asking @n1k0 to wade in here and see if he can point out any ways we're using casperjs incorrectly?

I don't think it's the framework that's necessarily at fault in the problem I reported, just a test which is intermittently failing in various ways, and I think we'll get that with any JS test framework.

gotdibbs commented 10 years ago

Definitely would like some other opinions and reviews of our existing structure. But my main point was that these intermittent issues keep popping up - and most of them would be solved if we did our functional testing synchronously me thinks.

n1k0 commented 10 years ago

Well I can tell you that switching to Selenium won't solve any problem with race conditions, wait conditions, even using a synchronous driver (especially in JavaScript where such a trick usually involves a bunch of not-so-reliable hacks.)

Intermittent failures are common when dealing with browser tests; on a Mozilla project I'm working on, we switched to Selenium but as you can see, intermittent failures are our daily bread. We even switched to Python for writing WebDriver tests because of the built-in synchronous API… no success, really. These browser automation things are hard, period.

The only advantage I see with Selenium is when you want to test for IE. But feel free to draw your own conclusions, of course :)

For the casperjs test problem raised here, ensure you're waiting for the correct context to be present before performing assertions. Same with Selenium btw.

ErisDS commented 10 years ago

@n1k0 Thanks for taking the time to come and give us your insight, it is very much appreciated.

EndangeredMassa commented 10 years ago

I agree with @n1k0. Sync/async isn't typically the problem.

I also encourage test writers to wait for the state they expect more often. It can solve a lot of these types of issues.

gotdibbs commented 10 years ago

Thanks for the thoughts @n1k0 and @EndangeredMassa !! This is exactly why I wanted a second opinion :). I was hopeful it might be a sync/async thing because the number of times we have had to up the timeout on a waitFor just because it sometimes runs a few seconds slower is bordering on crazy -- seems like others are having the same issue though, so it might just be something we live with.

Back to the issue at hand, I think from a glance #1312 might be related.

ErisDS commented 10 years ago

I think the attached PR will at least mitigate some of the failures that I have seen, although not the attached one. I am also going to limit us to Node 0.10 for now.

ErisDS commented 10 years ago

@gotdibs just FYI #1312 was in 0.3.3 but doesn't appear in master so it can't be that. I added a small fix but I pulled it cos although I think it fixes one small timeout - the tests are still failing regularly for me locally & on my Travis etc.

gotdibbs commented 10 years ago

@ErisDS You forgot a "b" :)

Have you noticed a common test failure? Or is it just the infinite loop we're talking about here?

ErisDS commented 10 years ago

Doh! There are a few different failures - infinite loop is most annoying one. The fix I did should resolve one. I need to start recording them instead of restarting in anger ;) Will do that today/tomorrow.

ErisDS commented 10 years ago

This is what I'm trying to fix with this PR: https://gist.github.com/ErisDS/54197ef8a9b746e2ddb1

ErisDS commented 10 years ago

Another failed test: https://gist.github.com/ErisDS/54368637f0e2e53d26f1

ErisDS commented 10 years ago

These are now running reliably again.

n1k0 commented 10 years ago

\o/