Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

migrate unit tests from test-unit to mini-test #256

Merged
merged 9 commits into from
Feb 14, 2015
Merged

migrate unit tests from test-unit to mini-test #256

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
50f55e0
unbundle testunit, bundle minitest
mroth Feb 12, 2015
5970558
remove useless git test
mroth Feb 12, 2015
97c577b
fix syntax issue
mroth Feb 12, 2015
7ad88e8
move plugins to own test file
mroth Feb 12, 2015
1c2e4bb
modern filename conventions
mroth Feb 12, 2015
6bc799a
switch cuke testunit assertions to rspec expects
mroth Feb 12, 2015
5b7ac96
address rubocop style issue with test
mroth Feb 12, 2015
926cb3d
switch to assert_equal to see fail condition
mroth Feb 13, 2015
918c845
ghetto fix for twitter plugin tests
mroth Feb 13, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Rakefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ end
Rake::Task[:build].prerequisites.unshift :fix_permissions

Rake::TestTask.new do |t|
t.pattern = 'test/test_*.rb'
t.pattern = 'test/*_test.rb'
end

Rake::FileUtilsExt.verbose(false)
Expand Down
6 changes: 3 additions & 3 deletions features/step_definitions/lolcommits_steps.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
end

Then /^I should be (prompted for|presented) "(.*?)"$/ do |_, prompt|
assert @stdout.read.to_s.include?(prompt)
expect(@stdout.read.to_s).to include(prompt)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we stick with the assert syntax in this file? Given we're using that style in the refactored plugin tests in this PR? Personally I prefer the assert MiniTest style over the 'expect to' grammar but this is just mho 😄

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I would have preferred that, but there is an 3-way incompatibility with minitest/cucumber/aruba because aruba depends on rspec-expectations (see 6bc799a), and none of the workarounds functioned in our case that I could get to work. Thankfully it's only for the few assertions in the cuke tests. From looking through the issues boards it sounds like this may be addressed by the next major version of cucumber.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I think I see why.. this is so we can remove the dependency on TestUnit assertions within step definitions? And use the rspec-expectations available through aruba..

It kinda sucks a bit that rspec leaks in here tho. I wonder if Aruba can work with MiniTest like assertions somehow.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, exactly. In theory we should be able to mixin MiniTest assertions the same way (although it requires a bit of boilerplate: https://github.com/cucumber/cucumber/wiki/Using-MiniTest) but that doesn't actually function if rspec functions are also loaded due to E_TOOMUCHMAGIC of different frameworks trying to do the right thing automatically.

That said even if it did work it introduces a larger amount of boilerplate than the 3-4 one line assertions.

I think its unlikely to be able to strip rspec-expectations out of Aruba, I think it uses a lot of its advanced functionality to write its ruleset. It may be possible to rewrite them all with minitest 5, which would be a pretty epic pull request for Aruba, but I suspect it would be quite an endeavor.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

end

When /^I enter "(.*?)" for "(.*?)"$/ do |input, field|
Expand All @@ -82,7 +82,7 @@
end

Then /^there should be (?:exactly|only) (.*?) (jpg|gif|pid)(?:s?) in "(.*?)"$/ do |n, type, folder|
assert_equal n.to_i, Dir["#{current_dir}/#{folder}/*.#{type}"].count
expect(n.to_i).to eq(Dir["#{current_dir}/#{folder}/*.#{type}"].count)
end

Then /^the output should contain a list of plugins$/ do
Expand Down Expand Up @@ -110,7 +110,7 @@

Then /^there should be (\d+) commit entries in the git log$/ do |n|
sleep 1 # let the file writing catch up
assert_equal n.to_i, `git shortlog | grep -E '^[ ]+\w+' | wc -l`.chomp.to_i
expect(n.to_i).to eq `git shortlog | grep -E '^[ ]+\w+' | wc -l`.chomp.to_i
end

Given /^I am using a "(.*?)" platform$/ do |platform_name|
Expand Down
2 changes: 0 additions & 2 deletions features/support/env.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
require 'aruba/cucumber'
require 'methadone/cucumber'
require 'open3'
require 'test/unit/assertions'
include Test::Unit::Assertions
require 'ffaker'
require 'fileutils'

Expand Down
2 changes: 1 addition & 1 deletion lolcommits.gemspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ Gem::Specification.new do |s|
s.add_development_dependency('ffaker', '~> 1.25.0')
s.add_development_dependency('travis', '~> 1.7.4')
s.add_development_dependency('coveralls', '~> 0.7.2')
s.add_development_dependency('test-unit', '~> 3.0.7')
s.add_development_dependency('minitest', '~> 5.5.1')

if RUBY_VERSION >= '1.9.3'
s.add_development_dependency('rubocop', '~> 0.28')
Expand Down
49 changes: 3 additions & 46 deletions test/test_lolcommits.rb → test/lolcommits_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
require 'coveralls'
Coveralls.wear!

require 'test/unit'
require 'ffaker'
require 'minitest/autorun'

# Loads lolcommits directly from the lib folder so don't have to create
# a gem before testing
$LOAD_PATH.unshift(File.join(File.dirname(__FILE__), '..', 'lib'))
Expand All @@ -12,50 +12,7 @@

include Lolcommits

class LolTest < Test::Unit::TestCase
def test_can_parse_git
assert_nothing_raised do
gi = GitInfo.new
assert_not_nil gi.message
assert_not_nil gi.sha
end
end

#
# issue #57, https://github.com/mroth/lolcommits/issues/57
#
def test_tranzlate
[['what the hell', '(WH|W)UT TEH HELL'], ['seriously wtf', 'SRSLEH WTF']].each do |normal, lol|
tranzlated = Lolcommits::Tranzlate.tranzlate(normal)
assert_match /^#{lol}/, tranzlated
end
end

#
# issue #136, https://github.com/mroth/lolcommits/issues/136
def test_lol_twitter_build_tweet
long_commit_message = Faker::Lorem.sentence(500)
plugin = Lolcommits::LolTwitter.new(nil)
max_tweet_size = 116
suffix = '... #lolcommits'

Lolcommits::LolTwitter.send(:define_method, :max_tweet_size, Proc.new { max_tweet_size })
assert_match "#{long_commit_message[0..(max_tweet_size - suffix.length)]}#{suffix}", plugin.build_tweet(long_commit_message)
end

def test_lol_twitter_prefix_suffix
plugin = Lolcommits::LolTwitter.new(nil)
Lolcommits::LolTwitter.send(:define_method, :max_tweet_size, Proc.new { 116 })
assert_match 'commit msg #lolcommits', plugin.build_tweet('commit msg')

plugin_config = {
'prefix' => '@prefixing!',
'suffix' => '#suffixing!'
}
Lolcommits::LolTwitter.send(:define_method, :configuration, Proc.new { plugin_config })
assert_match '@prefixing! commit msg #suffixing!', plugin.build_tweet('commit msg')
end

class LolTest < MiniTest::Test
#
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 its probably Ok to drop this test, I'd like to think most of the features would fail if GitInfo couldn't be read...

# issue #53, https://github.com/mroth/lolcommits/issues/53
# this will test the permissions but only locally, important before building a gem package!
Expand Down
52 changes: 52 additions & 0 deletions test/plugins_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# -*- encoding : utf-8 -*-
require 'coveralls'
Coveralls.wear!

require 'minitest/autorun'
require 'ffaker'

# Loads lolcommits directly from the lib folder so don't have to create
# a gem before testing
$LOAD_PATH.unshift(File.join(File.dirname(__FILE__), '..', 'lib'))
$LOAD_PATH.unshift(File.dirname(__FILE__))
require 'lolcommits'

include Lolcommits

class PluginsTest < MiniTest::Test
#
# issue #57, https://github.com/mroth/lolcommits/issues/57
#
def test_tranzlate
[['what the hell', '(WH|W)UT TEH HELL'], ['seriously wtf', 'SRSLEH WTF']].each do |normal, lol|
tranzlated = Lolcommits::Tranzlate.tranzlate(normal)
assert_match(/^#{lol}/, tranzlated)
end
end

#
# issue #136, https://github.com/mroth/lolcommits/issues/136
def test_lol_twitter_build_tweet
long_commit_message = Faker::Lorem.sentence(500)
plugin = Lolcommits::LolTwitter.new(nil)
max_tweet_size = 116
suffix = '... #lolcommits'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should add minitest/mock to the gemspec to allow us to cope with mocking better?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe! I messed around with trying to use the built in stub functionality in minitest, but couldn't get it to work in this case right off. If you want to take a stab at it feel free to push directly to this branch/PR. That said I think these tests may benefit from being better written in a different way entirely anyhow once we extract to gem plugins.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats true, these tests will sit outside the main gem in the end, lets punt on this for now.

Lolcommits::LolTwitter.send(:define_method, :max_tweet_size, Proc.new { max_tweet_size })
Lolcommits::LolTwitter.send(:define_method, :configuration, Proc.new { {} })
assert_equal "#{long_commit_message[0..(max_tweet_size - suffix.length)]}#{suffix}", plugin.build_tweet(long_commit_message)
end

def test_lol_twitter_prefix_suffix
plugin = Lolcommits::LolTwitter.new(nil)
Lolcommits::LolTwitter.send(:define_method, :max_tweet_size, Proc.new { 116 })
assert_match 'commit msg #lolcommits', plugin.build_tweet('commit msg')

plugin_config = {
'prefix' => '@prefixing!',
'suffix' => '#suffixing!'
}
Lolcommits::LolTwitter.send(:define_method, :configuration, Proc.new { plugin_config })
assert_equal '@prefixing! commit msg #suffixing!', plugin.build_tweet('commit msg')
end
end