Skip to content

Sean Luckett testing calculator submission#86

Open
SeanLuckett wants to merge 6 commits into
vikingeducation:masterfrom
SeanLuckett:master
Open

Sean Luckett testing calculator submission#86
SeanLuckett wants to merge 6 commits into
vikingeducation:masterfrom
SeanLuckett:master

Conversation

@SeanLuckett

Copy link
Copy Markdown

No description provided.

@remis1889 remis1889 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Write separate statements for each of the test cases. Missing test cases for stringify.

Comment thread spec/calculator_spec.rb
describe 'sqrt' do
it 'returns the square root of a number' do
expect(calc.sqrt(9)).to eq 3
expect(calc.sqrt(8)).to eq 2.83

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

the result for square root of a non-round argument should have 2 decimal places exactly

@SeanLuckett

Copy link
Copy Markdown
Author

I could certainly change the test from eq 3 to eq 3.00 to test this. Line 72 of the spec tests "stringify."

I disagree with writing separate statements for each test case. That is something taught to beginning testers, yes, but the important part is tests are understandable. Each expectation line is grouped under its intent. Putting each test case in its own block creates a lot of "noise" in the spec, making the reader have to hunt down and read a lot to understand what's going on.

I do appreciate the feedback! 👍

@remis1889

Copy link
Copy Markdown

I agree. Like you said, it is usually encouraged when you start learning about testing. Since this assignment is one of the first in RSpec unit, I made that comment. Once you start testing more complex applications it is better to group test cases so as to reduce the noise.

@remis1889

Copy link
Copy Markdown

Re: tests for stringify,
"If the stringify input is passed to a new calculator, the outputs of each of the computational functions above is turned into a string."
So, you are expected to write separate tests for each of the main operations. Something like this:

context 'with stringify input set as true' do
    let(:c_stringified) { Calculator.new(true) }

    describe '#add' do
      it 'should return a string' do
        expect(c_stringified.add(1,3)).to be_kind_of(String)
      end
    end

    describe '#subtract' do
      it 'should return a string' do
        expect(c_stringified.subtract(1,3)).to be_kind_of(String)
      end
    end
   # and so on..
   # ...
end

@remis1889

Copy link
Copy Markdown

Re: comment on line 55, square root of a perfect square should not have a decimal. Only non-round roots should have a decimal rounded up to 2 places, exactly. So, if sqrt(2) = 1.41421356237, the result should be 1.41. For this, in your test case, you should extract the part after the decimal and check if its length is 2. I hope I've made it clear.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants