Expanding test cases for Vehicle Purchase exercise in the Go track

Programming Go
1

I was working on the Vehicle Purchase exercise in the Go track today, and in the description I noticed the following requirement:
“For a rough estimate, assume if the vehicle is less than 3 years old, it costs 80% of the original price it had when it was brand new.”

However, when looking at the test cases, I found that there is no test case with an age of exactly 3. This means that the following check also works, while I expect it not to pass according to the requirements

if age < = 3 { // use 80% of original price }

While if age is exactly 3, it should adhere to the following requirement as stated in the description:
“If the vehicle is at least 3 years old but less than 10 years, it costs 70% of the original price.”

I would suggest to add the following test case to the TestCalculateResellPrice function in vehicle_purchase_test.go

{
	name:          "price is reduced to 70% for age 3",
	originalPrice: 40000,
	age:           3,
	expected:      28000,
},
1 Like
2

I would like to, if possible, try and create a PR for this myself to get some experience in contributing on Github.

1 Like
3

I’ve just read a similar post on the forums, where the solution was to edit the text rather than tests because of multiple reasons (breaking of existing solutions etc.), so perhaps that is a better option than adding a test case?

4

I wouldn’t mind adding the test case here as it’s reenforcing something that was already in the docs. But a Go maintainer (cc @junedev @andrerfcsantos) will need to weigh in on whether they want it added or not, in which case I’m sure they’d be happy for you to do the PR :slight_smile:

5

Changing test behavior in a way that is expected to break most/all existing tests is a high cost operation. Adding tests that are in line with existing requirements and probably won’t break most existing solutions but may break a small percentage is much lower cost.

6

@IsaacG Yeah that actually makes sense. In this case it would only impact the solutions where users check if age is lower than or equal to 3. Thanks!

7

@JesseKroon Feel free to open a PR with this and link it here, I will merge it :slight_smile:

We already have a test checking the age of 10, which is the other edge case for this exercise, so it makes sense to also check the age of 3.

I think potentially breaking existing solutions here is fine, as the new test is only making sure the solution meets the requirements. And since we are already testing the age of 10 edge case, if solutions pass this tests means the student probably understood well how to treat these edge cases, so it’s actually unlikely they made that mistake for the age of 3 edge case.

8

Cool, will do!

9

@andrerfcsantos here it is! Add testcase for TestCalculateResellPrice by jesse-kroon · Pull Request #2746 · exercism/go · GitHub

10

Merged. Thanks!