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

Specs: specs from numbers_exercises_spec have only one test case for some methods #76

Closed
3 of 4 tasks
edimossilva opened this issue Oct 12, 2022 · 3 comments · Fixed by #77
Closed
3 of 4 tasks
Assignees
Labels
Status: Needs Review This issue/PR needs an initial or additional review

Comments

@edimossilva
Copy link
Contributor

  • I have thoroughly read and understand The Odin Project Contributing Guide
  • The title of this issue follows the location for request: brief description of request format, e.g. Exercises: Add exercise on XYZ
  • I would like to be assigned this issue to work on it

1. Description of the Feature Request:
The methods add, subtract, multiply, divide and float_division have only one test case for them.

e.g. I suggested to a student to do the numbers exercise and the student did the following code:

def add(a, b)
  a = 1
  b = 2
  a + b
end

2. Acceptance Criteria:

  • Add additional specs for the methods add, subtract, multiply, divide and float_division to prevent hard coded solutions

e.g. Two specs for the sum method

   it 'adds two numbers' do
      expect(add(1, 2)).to eq(3)
      expect(add(4, 3)).to eq(7)
    end
@edimossilva edimossilva added the Status: Needs Review This issue/PR needs an initial or additional review label Oct 12, 2022
@rlmoser99
Copy link
Member

Great idea! I have a few things that would need to be taken into account. I've assigned this to you, but please let us know if you have any questions.

In rspec, you only want 1 expect statement per text. In addition, you want to test a variety of inputs, like positive or negative numbers. So I would suggest your example above to be more like the following:

   it 'adds two positive numbers' do
      expect(add(1, 2)).to eq(3)
    end

     it 'adds two negative numbers' do
      expect(add(-4, -3)).to eq(-7)
    end

In addition these changes would need to also be done on the solutions branch, so in order to completely fix this issue, we would expect to see 2 different Pull Requests. One to the main branch and one to the solutions branch (with all passing tests).

@ChargrilledChook
Copy link
Member

Hello @edimossilva , just checking in. Are you still interested in working on this? It looks like there were a few changes requested in your PR for this issue in #77

@ChargrilledChook
Copy link
Member

I've merged the linked PR which has closed this issue, but it would still be good to make these changes to the other types of exercises too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs Review This issue/PR needs an initial or additional review
Projects
Development

Successfully merging a pull request may close this issue.

3 participants