Skip to content
This repository was archived by the owner on Jan 3, 2023. It is now read-only.

Add tests for groceries exercise#82

Merged
illicitonion merged 1 commit intoCodeYourFuture:mainfrom
illicitonion:groceries-tests
May 9, 2021
Merged

Add tests for groceries exercise#82
illicitonion merged 1 commit intoCodeYourFuture:mainfrom
illicitonion:groceries-tests

Conversation

@illicitonion
Copy link
Copy Markdown
Member

I wasn't sure where to sit on the balance of hard-coding answers vs
giving potentially copyable parts of a solution in the tests... This is
where I settled :)

Starts to address CodeYourFuture/syllabus#230

I wasn't sure where to sit on the balance of hard-coding answers vs
giving potentially copyable parts of a solution in the tests... This is
where I settled :)
Copy link
Copy Markdown
Member

@SallyMcGrath SallyMcGrath left a comment

Choose a reason for hiding this comment

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

Looks great. It occurs to me that one thing you could do to minimise copying here would be to test with Set as if they just copy that back up top the test will fail as .includes is not a function on Set, but the comparison will still work the other way. IDK, might be overkill!

let allExpectedItems = new Set([...(Object.values(weeklyMealPlan))].flat());

@illicitonion
Copy link
Copy Markdown
Member Author

Yeah, I thought quite a bit about how to write the tests; the three things I was trading off were:

  1. Copy-ability - avoiding trainees just getting the answer from the test itself.
  2. Intelligibility - making sure the trainees can read the tests and understand what they're testing
  3. Error messages - making sure any error messages give enough information for the trainee to know what to think about next, rather than just that they got the answer wrong

I worried that using a Set suffers from being a construct the trainees haven't seen before, and also that generally performing equality assertions on Sets which give you useful error messages without the help of a test-framework is quite a lot of work... But hopefully we can work out our general approach here in our TDD meeting tonight! :)

Copy link
Copy Markdown
Contributor

@ChrisOwen101 ChrisOwen101 left a comment

Choose a reason for hiding this comment

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

I feel this is an acceptable answer for the time being which will be fixed when we move to using a testing framework :)

@illicitonion illicitonion merged commit 6f1ec5f into CodeYourFuture:main May 9, 2021
@illicitonion illicitonion deleted the groceries-tests branch May 9, 2021 10:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants