Manchester | 26-ITP-May | Szidonia Bodo | Sprint 3 | Implement and review test by bodoszidi · Pull Request #1520 · CodeYourFuture/Module-Structuring-and-Testing-Data

bodoszidi · 2026-07-16T18:47:17Z

Self checklist

I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
My changes meet the requirements of the task
I have tested my changes
My changes follow the style guide

Changelist

Created a new pull request only for implementation and review test

Liam310 · 2026-07-22T08:08:41Z

+assertEquals(isProperFraction(-8, 2), false);
+assertEquals(isProperFraction(4, 0), false);
+assertEquals(isProperFraction(0, 0), false);
+assertEquals(isProperFraction(-2, -2), false)


If you were to check the case of isProperFraction(-1, 2), what do we want to see as the answer? And would that test pass?

(Things get a little awkwardly mathsy with negative fractions, I recognise)

Since the function requires the numerator to be greater than 0, a negative numerator should return false.
The test isProperFraction(-1, 2) would pass because numerator > 0 will return false.

Liam310 · 2026-07-22T08:09:16Z

+assertEquals(getCardValue("J♦"), 10);
+assertEquals(getCardValue("7♥"), 7);
+assertEquals(getCardValue("A♥"), 11);
+assertEquals(getCardValue("8♥"), 8);


When writing a test suite, we want to try and cover all possible "behaviours" of our function. Here I can see you've covered:

Hearts

Spades

Diamonds

Numbered cards

Jacks

Aces

Looking at that list, what have you not covered?

I haven't covered Clubs (♣) or the other face cards (Q and K). Although they are handled by the same logic as Jacks, adding test cases for them would improve coverage and help catch any mistakes in those specific conditions.

Still looks like there's one suit we haven't covered yet 👀

Added the missing suit and new test cases.

Liam310 · 2026-07-22T08:10:20Z

+
+test(`should return false when denominator is bigger than the numerator`, () => {
+  expect(isProperFraction(5, -2)).toEqual(false);
+  expect(isProperFraction(-1, 0)).toEqual(false);


expect(isProperFraction(-1, 0)).toEqual(false);

This test case, which test does it belong in? What grouping of behaviours does it fall into?

Should be in a different case to test negative numbers - updated the test file.

Two things:

I think there is an earlier test description that really best describes the isProperFraction(-1, 0) case!

Numerator/denominator being negative technically doesn't determine whether a fraction is proper or improper. For example, isProperFraction(-2, 5) should actually be true because 2 is smaller than 5. How you could account for this?

Liam310 · 2026-07-22T08:20:12Z

Hey @bodoszidi! 👋

Good work on your first journey into using tests. Writing tests from scratch is not easy, it's a skill that takes a while to develop but there's a lot of good stuff in here.

My comments are mostly about focussing on how to organise our test cases. The main things we want to be doing are:

Trying to achieve good test coverage, which means "have we considered all possible scenarios here?"
Defining tests according to "behaviours" of the function, which means thinking about "how does the function behave with a certain type of input?" and figuring out how to group those inputs when relevant. A good rule of thumb is: if they're all handled by the same bit of logic in the function, they should probably be grouped as part of the same behaviour.

Please do respond to any comments with answers or further questions if you aren't sure what I'm talking about! 😄

Removed redundant comments from test cases for clarity.

…ional tests Refactor variable declaration to use const for removeSuit and add test cases for Queen and King.

Added a test case to check for proper fraction with positive numbers.

Liam310 · 2026-07-28T08:53:00Z

@bodoszidi Good improvements! Just a couple of lingering comments around handling negative fractions 🙂

bodoszidi added 6 commits July 16, 2026 19:30

Implemented the function and tested

f0ca3d7

added test cases

5ad372d

implemented function and tested

5c0d472

added jest test cases

b28e30e

implemented function and tested

241ef30

added jest test cases

f0b6faa

This comment has been minimized.

Sign in to view

bodoszidi added 📅 Sprint 3 Assigned during Sprint 3 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Module-Structuring-And-Testing-Data The name of the module. labels Jul 16, 2026

This comment has been minimized.

Sign in to view

github-actions Bot removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jul 16, 2026

This comment has been minimized.

Sign in to view

bodoszidi added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jul 16, 2026

bodoszidi mentioned this pull request Jul 16, 2026

26-ITP-May | Structuring and testing data | Szidonia Bodo | bodoszidi/My-Coursework-Planner#51

Open

Liam310 added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Jul 21, 2026

Liam310 reviewed Jul 22, 2026

View reviewed changes

Comment thread Sprint-3/1-implement-and-rewrite-tests/implement/2-is-proper-fraction.js Outdated

Liam310 reviewed Jul 22, 2026

View reviewed changes

Comment thread Sprint-3/1-implement-and-rewrite-tests/implement/3-get-card-value.js

Liam310 reviewed Jul 22, 2026

View reviewed changes

Comment thread Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/1-get-angle-type.test.js Outdated

Liam310 reviewed Jul 22, 2026

View reviewed changes

Comment thread Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/3-get-card-value.test.js

Liam310 added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Jul 22, 2026

bodoszidi added 2 commits July 24, 2026 10:47

removed unnecessary comments

10d37a8

Removed redundant comments from test cases for clarity.

updated the variable let to const inside the function and added addit…

1230568

…ional tests Refactor variable declaration to use const for removeSuit and add test cases for Queen and King.

bodoszidi added 3 commits July 24, 2026 11:10

Simplify isProperFraction function return statement

26a0fed

Add test to handle negative numbers

9c84e3d

Added a test case to check for proper fraction with positive numbers.

Grouped tests cases

cb7b6ef

bodoszidi added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Jul 24, 2026

updated test cases for the function

e569ea9

Uh oh!

Uh oh!

Conversation

bodoszidi commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelist

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Uh oh!

Liam310 Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Liam310 commented Jul 22, 2026

Uh oh!

Liam310 commented Jul 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bodoszidi commented Jul 16, 2026 •

edited

Loading

Liam310 Jul 22, 2026 •

edited

Loading