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.

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

Suggestion: Migrate tests to RSpec #92

Closed
MadsNielsen opened this issue Mar 3, 2017 · 5 comments
Closed

Suggestion: Migrate tests to RSpec #92

MadsNielsen opened this issue Mar 3, 2017 · 5 comments
Assignees
Milestone

Comments

@MadsNielsen
Copy link
Member

I looked at our current tests for PAC and it seems that there is a distinct lack of feedback when you run the tests. The current pipeline does not summarize the report or print the results in any meaningful way.

Futhermore the tests are hard to understand, very long, and do not test actual use cases. I spent like 30 minutes write a test that demonstrates how RSpec works

require_relative '../lib/core'

RSpec.describe Core, "#read_settings" do	
	context "when settings file exists" do 
		it "returns the contents of the found settings file" do
			settings_file = File.join(File.dirname(__FILE__), '../default_settings.yml')
			arguments = { '--settings' => "#{settings_file}" }
			file_parsed = Core.read_settings_file(arguments)
			expect(file_parsed).not_to be_nil
		end
	end
	context "when settings file does not exist" do 
		it "throws an exception with description" do 
			settings_file = File.join(File.dirname(__FILE__), '../../default_settings_not.yml')
			arguments = { '--settings' => "#{settings_file}" }
			expect { Core.read_settings_file(arguments) }.to raise_error(RuntimeError)	
		end
	end
end

When you run a test like this in rspec you get an output like so:

Core#read_settings
  when settings file exists
    returns the contents of the found settings file
  when settings file does not exist
    throws an exception with description

It's alot simpler and you get a REALLY nice report if you run rspec --format=html

Is this something we want to do?

@buep
Copy link
Contributor

buep commented Mar 3, 2017

I think that sound like a good idea. We have other issues on improving tests also, need maybe a coordinated effort on it.
Beside the nice report you mention is possible, we would like some way of graphing or counting number of tests for the functional ones to show graph in the build systems.
Unit test is something else, but we also want count and then coverage if we have real unit test.

@buep buep added this to the backlog milestone Mar 14, 2017
MadsNielsen added a commit to MadsNielsen/Praqmatic-Automated-Changelog that referenced this issue Mar 24, 2017
@buep
Copy link
Contributor

buep commented Apr 4, 2017

Will await a test strategy.

@buep
Copy link
Contributor

buep commented Apr 6, 2017

@MadsNielsen I'm okay using rspec, but before further changes we need to have a plan, on how and when as well as which kind of test we should implement. I still prefer as much unit testing as possible as this runs fast.

I'll call you and @michaelmadsen for a meeting, as you two seems the most eager to work with Ruby so we together can form a test strategy for PAC.

@buep buep self-assigned this Apr 6, 2017
@buep
Copy link
Contributor

buep commented May 1, 2017

We have decided to move to rspec today and have made some testing guidelines and the first rspec tests.

@buep buep closed this as completed May 1, 2017
@buep
Copy link
Contributor

buep commented May 1, 2017

See #98

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants