Update specs to use modern style #321
Conversation
Updated the specs to reflect a cleaner style of rspec testing. For info about spec style visit http://betterspecs.org/
|
Currently having an issue with Rubocop: Rubocop is complaining about the length of the Otherwise this file is ready for review. Let me know if the style is suitable and I will proceed to work on more specs. |
spec/io/sql_data_source_spec.rb
Outdated
| Daru::IO::SqlDataSource.make_dataframe(Object.new, query) | ||
| }.to raise_error(ArgumentError) | ||
| end | ||
| subject(:df) { Daru::IO::SqlDataSource.make_dataframe(active_record_connection, query) } |
There was a problem hiding this comment.
This could be written once, at the very beginning, as subject(:df) { described_class.make_dataframe(source, query) },
and then you just let(:source) to different values in different contexts.
spec/io/sql_data_source_spec.rb
Outdated
| end | ||
| subject(:df) { Daru::IO::SqlDataSource.make_dataframe(active_record_connection, query) } | ||
| it { is_expected.to be_a(Daru::DataFrame) } | ||
| it { expect(df.nrows).to eq 2 } |
There was a problem hiding this comment.
its(:nrows) { is_expected.to eq 2 }
spec/io/sql_data_source_spec.rb
Outdated
| it { is_expected.to be_a(Daru::DataFrame) } | ||
| it { expect(df.nrows).to eq 2 } | ||
| it { expect(df.row[0][:id]).to eq 1 } | ||
| it { expect(df.row[0][:name]).to eq 'Homer' } |
There was a problem hiding this comment.
it { expect(df.row[0]).to have_attributes ...
Better communicates that we are testing the same object (first row).
spec/io/sql_data_source_spec.rb
Outdated
| it { | ||
| expect { | ||
| Daru::IO::SqlDataSource.make_dataframe(Object.new, query) | ||
| }.to raise_error(ArgumentError) |
There was a problem hiding this comment.
If you'd follow my suggestion about common subject, this context could be turned into
let(:source) { Object.new }
it { expect { subject }.to raise_error(ArgumentError) }And I'd suggest to check error message too.
And yes, block length limitation has no value for specs. It could be disabled in local |
To keep the spec dry, we reuse the subject and set a new source or query in every context
|
@zverok I made the requested changes. I had an issue testing the It seems that the |
spec/io/sql_data_source_spec.rb
Outdated
| @@ -14,70 +14,58 @@ | |||
| ActiveRecord::Base.connection | |||
| end | |||
|
|
|||
| let(:dat_file) do | |||
| 'spec/fixtures/bank2.dat' | |||
| end | |||
There was a problem hiding this comment.
It is used only in one context -- therefore should be initialized there.
There was a problem hiding this comment.
And same about all others (dbi_handle, active_record_connection). In fact, you can just do in an appropriate context:
let(:source) { DBI.connect("DBI:SQLite3:#{db_name}") }...and that's it.
spec/io/sql_data_source_spec.rb
Outdated
| let(:source) { dbi_handle } | ||
| it { is_expected.to be_a(Daru::DataFrame) } | ||
| it { expect(df.row[0]).to have_attributes(id: 1) } | ||
| it { expect(df.row[0]).to have_attributes(age: 20) } |
There was a problem hiding this comment.
Join those two: it { expect(df.row[0]).to have_attributes(id: 1, age: 20) }, it is logically just one test "we have right row here".
|
@zverok Thank you for the feedback. I moved the declarations into the contexts. The ActiveRecord connection is the default source since it is used twice. So far we have gone from 83 lines to 56 . |
Updated the specs to reflect a cleaner style of rspec testing. For info
about spec style visit http://betterspecs.org/