Skip to content

London | 26-ITP-Sep | Mars Adesina | Sprint 2 | Implement and rewrite tests - #1718

Open
marscancode wants to merge 28 commits into
CodeYourFuture:mainfrom
marscancode:coursework/sprint-3-implement-and-rewrite
Open

marscancode wants to merge 28 commits into
CodeYourFuture:mainfrom
marscancode:coursework/sprint-3-implement-and-rewrite

Conversation

@marscancode

Copy link
Copy Markdown

Learners, PR Template

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

Task code

CYF-1059

Changelist

I implemented the three functions:

  1. getAngleType
  2. isProperFraction
  3. getCardValue
    I also created tests in Node and Jest for all three functions.

@marscancode marscancode added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. 📅 Sprint 2 Assigned during Sprint 2 of this module Module-Structuring-And-Testing-Data The name of the module. labels Oct 15, 2026
@cjyuan cjyuan 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 Oct 23, 2026
Comment thread implement-and-rewrite-tests/implement/2-is-proper-fraction.js Outdated
@@ -1,3 +1,3 @@
import { isProperFraction } from "../implement/2-is-proper-fraction.js";

// TODO: Write tests in Jest syntax to cover all combinations of positives, negatives, zeros, and other categories.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The test cases in this script do not yet include cases involving negative numbers.

Are $\frac{-2}{3}$, $\frac{2}{-3}$ (which equals to $-\frac{2}{3}$) and $\frac{-2}{-3}$ (which equals to $\frac{2}{3}$) proper fractions? What is the definition of proper fraction?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I've now included tests for negative numbers. Thankyou!

test("Arbitrary non-card string", () => {
expect(() => getCardValue("invalid")).toThrow(
/Expected a valid number followed by a suit, but got "invalid"/
);

@cjyuan cjyuan Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why test the error message as

    expect(() => getCardValue("invalid")).toThrow(
      /Expected a valid number followed by a suit, but got "invalid"/
    );

instead of as

    expect(() => getCardValue("invalid")).toThrow(
      'Expected a valid number followed by a suit, but got "invalid"'
    );

?

@marscancode marscancode Oct 25, 2026 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

For some reason, I thought this was the syntax used when using .toThrow in jest but i will update with the correct syntax now. Thankyou!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you use an AI to find out

  • The difference and similarity between these two types of argument when using Jest's expect.toThrow()?
  • How to perform an exact match of the error message with Jest expect.toThrow()?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tests are very comprehensive. Excellent job.

@cjyuan cjyuan 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 Oct 23, 2026
@marscancode marscancode 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 Oct 25, 2026

@cjyuan cjyuan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changes look good.

@@ -1,3 +1,4 @@
import { get } from "node:http";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why import get? Where is it used?

Comment on lines +25 to +35
test(`proper fraction with a negative numerator but positive denominator`, () => {
expect(isProperFraction(-2, 3)).toEqual(true);
});

test(`proper fraction with a positive numerator but negative denominator`, () => {
expect(isProperFraction(2, -3)).toEqual(true);
});

test(`proper fraction with both a negative numerator and denominator`, () => {
expect(isProperFraction(-2, -3)).toEqual(true);
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note: Rather than writing a separate test for each sign combination, you could consider grouping inputs that exercise the same behaviour into one test, under a test description that is applicable to all. For example,

test("should return true when abs(numerator) < abs(denominator)", () => {
  expect(isProperFraction(1, 2)).toEqual(true);
  expect(isProperFraction(-1, 2)).toEqual(true);
  expect(isProperFraction(1, -2)).toEqual(true);
  expect(isProperFraction(-1, -2)).toEqual(true);
});

Note: Use of widely recognised pseudo-code and math notations like abs(...) or | ... | in the descriptions is an usually accepted.

No change required

@cjyuan cjyuan added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Oct 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed. Module-Structuring-And-Testing-Data The name of the module. 📅 Sprint 2 Assigned during Sprint 2 of this module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants