Skip to content

West Midlands | 26-Jan-ITP | Fida H Ali Zada | Sprint 3 | Implement and Rewrite Tests#1129

Open
alizada-dev wants to merge 12 commits intoCodeYourFuture:mainfrom
alizada-dev:coursework/sprint-3-implement-and-rewrite
Open

West Midlands | 26-Jan-ITP | Fida H Ali Zada | Sprint 3 | Implement and Rewrite Tests#1129
alizada-dev wants to merge 12 commits intoCodeYourFuture:mainfrom
alizada-dev:coursework/sprint-3-implement-and-rewrite

Conversation

@alizada-dev
Copy link

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

Changelist

Completed the Sprint 3 Implement and Rewrite Tests. I hope to see comments on my work so I can improve. Thank you.

@alizada-dev alizada-dev added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Mar 2, 2026
// https://jestjs.io/docs/expect#tothrowerror

// Number Cards (2-10)
test(`should return the number (numeric value)`, () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

How about ... the numeric value of number cards?

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Mar 11, 2026
@alizada-dev alizada-dev 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 Mar 11, 2026
Comment on lines +12 to +38
test(`should return true when (numerator < denominator)`, () => {
expect(isProperFraction(0, 1)).toEqual(true);
})

test(`should return true when (numerator < denominator)`, () => {
expect(isProperFraction(-6, -3)).toEqual(true);
})

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

test(`should return false when (numerator > denominator)`, () => {
expect(isProperFraction(2, 1)).toEqual(false);
})

test(`should return false when (numerator == denominator)`, () => {
expect(isProperFraction(0, 0)).toEqual(false);
})

test(`should return false when (numerator > denominator)`, () => {
expect(isProperFraction(-1, -2)).toEqual(false);
})

test(`should return false when (numerator > denominator)`, () => {
expect(isProperFraction(0, -1)).toEqual(false);
}) No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you have updated the function, can you also update this test script?

You can use notations such as | ... | or abs( ... ) in the test description, and can probably group some of the test cases into the same group.

Copy link
Author

Choose a reason for hiding this comment

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

Ah, got it, sir! I totally forgot the test cases. Now I fixed them.

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Mar 11, 2026
@alizada-dev alizada-dev 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 Mar 12, 2026
Copy link
Contributor

@cjyuan cjyuan left a comment

Choose a reason for hiding this comment

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

Changes look good.

Comment on lines +23 to +25
test(`should return false when (numerator == denominator)`, () => {
expect(isProperFraction(0, 0)).toEqual(false);
}) No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

This test would have missed detecting a bug in this implementation:

function isProperFraction(numarator, denominator) {
  if (denominator == 0) return false;
  return Math.abs(numarator) <= Math.abs(denominator);
}

expect(isProperFraction(2, 1)).toEqual(false);
expect(isProperFraction(-2, -1)).toEqual(false);
})

Copy link
Contributor

Choose a reason for hiding this comment

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

You have more comprehensive test data in Sprint-3/1-implement-and-rewrite-tests/implement/2-is-proper-fraction.js.

@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 Mar 12, 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants