Bugfix/feed service tests

[wilkie]

The original tests weren't... um... doing much. :) I mean... look at:

https://github.com/hotsh/rstat.us/blob/master/test/services/feed_service_test.rb#L62-L64

Stubs out every method, asserts that it calls the stub and gets the value stubbed. That pretty much tests ruby's precedence order for the || operator. Apparently, there was this urge in the original developer to stub out private methods. But since FeedService is entirely private methoded, that's not ideal. For instance: FeedService#find_feed_by_remote_url is never called by a test.

Private methods are forced integrations. They must be invoked to test behavior of the public methods. Otherwise, they will never be covered. I did, however, remove db dependency.

I also snuck in the mocha deprecation fix with a mocha update here because this was one of the files which involved the issue, I believe.

Speed

I'm trying to refactor tests in sight of #706 and here are the benchmarks for test/services/feed_service_test.rb

Original

$ rake test:file[test/services/feed_service_test.rb]
test/services/feed_service_test.rb
Rack::File headers parameter replaces cache_control after Rack 1.5.
Run options: --seed 19878

# Running tests: .....

Finished tests in 0.200287s, 24.9641 tests/s, 24.9641 assertions/s.

After ensuring internal codepaths are stubbed

$ rake test:file[test/services/feed_service_test.rb]
test/services/feed_service_test.rb
Rack::File headers parameter replaces cache_control after Rack 1.5.
Run options: --seed 31669

# Running tests: .....

Finished tests in 0.195127s, 25.6243 tests/s, 25.6243 assertions/s.

Stubs out db calls, external codepaths

$ rake test:file[test/services/feed_service_test.rb]
test/services/feed_service_test.rb
Rack::File headers parameter replaces cache_control after Rack 1.5.
Run options: --seed 24722

# Running tests: .....

Finished tests in 0.202075s, 24.7433 tests/s, 29.6920 assertions/s.

I didn't run them on the same seed, but still, no change! These tests are as simple as they can be. I have entire projects with around 200 assertions that run faster than these 5 tests. There is apparently a high upfront overhead of, apparently, 0.2s.

[wilkie]

Removing TestHelper

$ rake test:file[test/services/feed_service_test.rb]
test/services/feed_service_test.rb
Rack::File headers parameter replaces cache_control after Rack 1.5.
Run options: --seed 41627

# Running tests: .....

Finished tests in 0.015902s, 314.4225 tests/s, 377.3070 assertions/s.

Well. This is starting to make a lot of sense. Let's just get rid of our database dependencies in unit tests altogether... should reduce the overhead considerably.

[wilkie]

Removing Require of test_helper.rb

By taking advantage of the lack of requires and going more old school mocking we can remove requiring test_helper altogether:

require "minitest/autorun"
require 'mocha/setup'

class User; end
class Feed; end
class SubscriberToFeedDataConverter; end

require_relative '../../app/services/feed_service'

...

Original refactoring through #743 thus far:

$ time rake test:file[test/services/feed_service_test.rb]
Run options: --seed 45013

Finished tests in 0.015600s, 320.5111 tests/s, 384.6133 assertions/s.

real    0m18.919s
user    0m18.087s
sys     0m0.780s

Not requiring test_helper.rb along with the rest of #743:

$ time rake test:file[test/services/feed_service_test.rb]
Run options: --seed 55063

Finished tests in 0.018919s, 264.2831 tests/s, 317.1397 assertions/s.

real    0m7.612s
user    0m7.283s
sys     0m0.310s

Conclusion: requiring test_helper.rb adds considerable external overhead: ~11s.

[steveklabnik]

This needs a rebase after I merged #745.

[carols10cents]

looks good to me! :+1:

sorry for letting this sit out here in merge purgatory for so long :(