Skip to content

Conversation

@hendelSab
Copy link

No description provided.

@gemtechd gemtechd self-requested a review July 13, 2025 12:51
Copy link
Owner

@gemtechd gemtechd left a comment

Choose a reason for hiding this comment

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

See that in all your tests, the each test executes only one function
if needed, update the code iteself

});

describe('calculateJunctionPoint', () => {
it('מחזירה true עבור קווים חופפים', () => {
Copy link
Owner

Choose a reason for hiding this comment

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

no hebrew...

const l2 = new Line({ point1: new Point({ x: 2, y: 2 }), point2: new Point({ x: 3, y: 3 }) });
l2.calculateSlope();
l2.calculateNOfLineFunction();
expect(calculateJunctionPoint(l1, l2)).toBe(true);
Copy link
Owner

Choose a reason for hiding this comment

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

each test has to execute one function, and test it
you can update the code self that it ahould work as expected (it was written in the README file)

@hendelSab hendelSab requested a review from gemtechd July 13, 2025 13:07
Copy link
Owner

@gemtechd gemtechd left a comment

Choose a reason for hiding this comment

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

Read the code review remarks and fix the code

import Line from '../modules/ecs6-class/line';
import Point from '../modules/ecs6-class/point';

// modules/geometry-calculation.test.ts
Copy link
Owner

Choose a reason for hiding this comment

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

don't push remarks

// modules/geometry-calculation.test.ts

describe('calculateDistance', () => {
it('מחזירה 0 עבור אותה נקודה', () => {
Copy link
Owner

Choose a reason for hiding this comment

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

hebrew?

@hendelSab
Copy link
Author

Correcting errors in the test

@hendelSab
Copy link
Author

hendelSab commented Jul 14, 2025 via email

it('returns true for a point on the line', () => {
const l = new Line({ point1: new Point({ x: 0, y: 0 }), point2: new Point({ x: 2, y: 2 }) });
l.calculateSlope();
l.calculateNOfLineFunction();
Copy link
Owner

Choose a reason for hiding this comment

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

still caling two functions

Copy link
Owner

Choose a reason for hiding this comment

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

The only function that should be executed in this test is the isPointOnLine function, nothing more

Copy link
Owner

@gemtechd gemtechd left a comment

Choose a reason for hiding this comment

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

Still 2 functions on a test

const p2 = new Point({ x: 1, y: 3 });
const line = new Line({ point1: p1, point2: p2 });
line.calculateSlope();
line.calculateNOfLineFunction();
Copy link
Owner

Choose a reason for hiding this comment

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

2 functions...

it('returns true for a point on the line', () => {
const l = new Line({ point1: new Point({ x: 0, y: 0 }), point2: new Point({ x: 2, y: 2 }) });
l.calculateSlope();
l.calculateNOfLineFunction();
Copy link
Owner

Choose a reason for hiding this comment

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

The only function that should be executed in this test is the isPointOnLine function, nothing more

Copy link
Owner

@gemtechd gemtechd left a comment

Choose a reason for hiding this comment

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

See the comment about the mocks
It is all over in the project

l1.calculateNOfLineFunction();
const l2 = new Line({ point1: new Point({ x: 2, y: 2 }), point2: new Point({ x: 3, y: 3 }) });
l2.calculateNOfLineFunction();
expect(calculateJunctionPoint(l1, l2)).toBe(true);
Copy link
Owner

Choose a reason for hiding this comment

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

did you use a different function in the calculateJunctionPoint function
then you have to mock it
you have to mock the Line class
How to mock an ES-Class was one of your questions that you had to answer....

Copy link
Author

Choose a reason for hiding this comment

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

I tried to do a mock and got confused. Maybe the teacher can help and if I'm on the right track?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants