add tests #13

lev527 · 2024-07-21T08:32:54Z

No description provided.

gemtechd

See the comments

gemtechd · 2024-07-21T09:32:36Z

modules/ecs6-class/line.js

+        if(typeof(n)!='number'&& typeof(n)!='undefined'){
+            throw Error('The n should have a number')
+        }
+        if(typeof(slope)!='number'&& typeof(slope)!='undefined'){


use the three equals operator

gemtechd · 2024-07-21T09:33:22Z

modules/ecs6-class/line.js


-    getPointOnXAsis() {
-        return this.getPointByY(0)
+    getPointOnXAsis(func=this.getPointByY(0)) {


Why did you add this code, no need

gemtechd · 2024-07-21T09:34:03Z

modules/geometry-calculation.js


 const calculateDistance = (point1, point2) => {
+    if(typeof(point1)!='object'||typeof(point1)!='object'){
+        throw Error('The value is of an invalid type')


Which of the point is invalid?
You can do a better check

gemtechd · 2024-07-21T09:35:27Z

modules/geometry-calculation.js

+        throw Error('The value is of an invalid type')
+    }
+    if(!point1.x||!point2.x){
+        throw Error('The value is of an invalid type')


And what with the y value?
check in your first if not only the object but deeper

gemtechd · 2024-07-21T09:36:17Z

modules/geometry-calculation.js

-const calculateJunctionPoint = (line1, line2) => {
+const calculateJunctionPoint = (line1, line2,func) => {
+    if(typeof(line1)!='object'||typeof(line2)!='object'){
+        throw Error('The value is of an invalid type')


Check deeper and not only object

modules/geometry-calculation.js

gemtechd · 2024-07-21T09:37:59Z

modules/geometry-calculation.js

 }

-const isPointOnLine = (line, point) => {
+const isPointOnLine = (line, point,func1,func2) => {


don't send this funcs, in the tests use real mocks

package.json

tests/modules/geometry-calculation.test.js

gemtechd

Update your code, and see if you hve 100 coverage

gemtechd · 2024-07-23T10:49:03Z

modules/ecs6-class/line.js

-        if(typeof(point1.x)!='number'||typeof(point2.x)!='number'){
+    constructor({ point1 = new Point(), point2 = new Point(), n = undefined, slope = undefined } = {}) {
+        if (!(point1 instanceof Point) || !(point2 instanceof Point)) {
            throw Error('The points should have Point type')


Which one threw the error, point1 or point2?

modules/ecs6-class/line.js

gemtechd · 2024-07-23T10:51:12Z

modules/ecs6-class/line.js

-        this.n = this.point1.y - this.slope * this.point1.x
+    calculateNOfLineFunction() {
+
+        return this.n = this.point1.y - this.slope * this.point1.x


What happens if the slope is not calculated yet?

you really don't need to do it each time, only in case that it is not calculated

gemtechd · 2024-07-23T10:51:43Z

modules/ecs6-class/line.js

+            throw Error('The value is of an invalid type')
+        }
        let y = this.slope * x + this.n
        return new Point({ x, y })


what happens if the slope or the line are not calculated yet?

first check what is the n value
you can call only the n, it will calculate the slope if the slope is undefined

gemtechd · 2024-07-23T10:52:15Z

modules/ecs6-class/line.js

+        if (typeof (y) !== 'number') {
+            throw Error('The value is of an invalid type')
+        }
        let x = (y - this.n) / this.slope;


what happens when the slope is not calculated or is zero?

The same as the previous remark

gemtechd · 2024-07-23T10:56:08Z

tests/modules/ecs6-class/line.test.js

-        let point2 = new Point({ x: 3, y: 8 });
-        let line1 = new Line({ point1, point2, n: 9.5, slope: -0.5 });
-        expect(line1.getPointOnXAsis(mock)).toEqual({ x: 19, y: 0 })
+        test('It should be returned if in the call to the function the function getPointByY was called', () => {


use the it function

gemtechd · 2024-07-23T10:57:21Z

tests/modules/ecs6-class/line.test.js

+
+const getPointByXMock = jest
+    .spyOn(Line.prototype, 'getPointByX')
+


where do you restore the mocks?
I prefer using the jest.mock and not the spyOn
but if you use it, restore it

use this mocks inside the test where they are needed

gemtechd · 2024-07-23T10:58:43Z

tests/modules/ecs6-class/line.test.js

@@ -39,46 +45,70 @@ describe('Check calculateNOfLineFunction function', () => {
 })

 describe('Check getPointOnXAsis function', () => {


You are testing only if the mock was called, but where are the tests about the values of the function??

gemtechd · 2024-07-23T11:05:53Z

tests/modules/ecs6-class/line.test.js

Call the test functions with the it expression

gemtechd · 2024-07-23T11:07:39Z

tests/modules/geometry-calculation.test.js

+const calculateNOfLineFunctionMock = jest
+    .spyOn(Line.prototype, 'calculateNOfLineFunction').mockImplementation()
+    .mockReturnValue(9)
+


I think it's better to mock them each time again and restore
you cannot hold how many times you'll use them
and each time you can expect a different value

Do I need to write the mock function many times?
And each time give a different value?

mock the class once with jest.mock, and each time mocktheReturnValue for each function

gemtechd

See the comments
the changed is the code are important

gemtechd · 2024-07-24T18:39:20Z

modules/ecs6-class/line.js

-        this.n = this.point1.y - this.slope * this.point1.x
+    calculateNOfLineFunction() {
+
+        return this.n = this.point1.y - this.slope * this.point1.x


you really don't need to do it each time, only in case that it is not calculated

gemtechd · 2024-07-24T18:40:29Z

modules/ecs6-class/line.js

+            throw Error('The value is of an invalid type')
+        }
        let y = this.slope * x + this.n
        return new Point({ x, y })


first check what is the n value
you can call only the n, it will calculate the slope if the slope is undefined

gemtechd · 2024-07-24T18:40:47Z

modules/ecs6-class/line.js

+        if (typeof (y) !== 'number') {
+            throw Error('The value is of an invalid type')
+        }
        let x = (y - this.n) / this.slope;


The same as the previous remark

gemtechd · 2024-07-24T18:41:39Z

modules/geometry-calculation.js

+    line1.calculateSlope()
+    line2.calculateSlope()
+    line1.calculateNOfLineFunction()
+    line2.calculateNOfLineFunction()


the same here, we don't call a function if it is not necessary

But the teacher wrote that you should check if the slope and n are correct.
If the user entered wrong values?
and also
If the user entered only n and no slope - the slope will not be calculated.
Therefore, it should be checked separately.

gemtechd · 2024-07-24T18:42:42Z

modules/geometry-calculation.js

-    }
-
+    proxyLine.slope=proxyLine.calculateSlope()
    if (line.slope === proxyLine.slope) {


the calculateSlope returns a value???
you are updating the slope into undefined

gemtechd · 2024-07-24T18:46:43Z

modules/geometry-calculation.js

+    proxyLine.slope=proxyLine.calculateSlope()
    if (line.slope === proxyLine.slope) {
-        proxyLine.calculateNOfLineFunction()
+        proxyLine.n=proxyLine.calculateNOfLineFunction()


the same here, the n will be updated into undefined

if you did it for the mocks, in the mocks you can do whatever you want, but here you need to use the function as it has to be

gemtechd · 2024-07-24T18:47:07Z

package.json

+    "test:coverage": "jest --coverage"
  },
  "dependencies": {
    "jest": "^29.7.0"


gemtechd · 2024-07-24T18:48:02Z

tests/modules/ecs6-class/line.test.js

+
+const getPointByXMock = jest
+    .spyOn(Line.prototype, 'getPointByX')
+


use this mocks inside the test where they are needed

gemtechd · 2024-07-24T18:51:05Z

tests/modules/geometry-calculation.test.js

+const calculateNOfLineFunctionMock = jest
+    .spyOn(Line.prototype, 'calculateNOfLineFunction').mockImplementation()
+    .mockReturnValue(9)
+


mock the class once with jest.mock, and each time mocktheReturnValue for each function

gemtechd · 2024-07-25T08:29:58Z

good, leave it like this and continue ‫בתאריך יום ה׳, 25 ביולי 2024 ב-10:24 מאת ‪lev527‬‏ <‪ ***@***.***‬‏>:‬

…

***@***.**** commented on this pull request. ------------------------------ In modules/geometry-calculation.js <#13 (comment)>: > const distance = Math.sqrt(distanceX + distanceY); return distance; } const calculateJunctionPoint = (line1, line2) => { + if (!(line1 instanceof Line) && !(line2 instanceof Line)) { + throw Error('The values are of an invalid type') + } + if (!(line1 instanceof Line)) { + throw Error('The first value is of an invalid type') + } + if (!(line2 instanceof Line)) { + throw Error('The second value is of an invalid type') + } + line1.calculateSlope() + line2.calculateSlope() + line1.calculateNOfLineFunction() + line2.calculateNOfLineFunction() But the teacher wrote that you should check if the slope and n are correct. If the user entered wrong values? and also If the user entered only n and no slope - the slope will not be calculated. Therefore, it should be checked separately. — Reply to this email directly, view it on GitHub <#13 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BHKPZ653IUSXTGTUARUWFZTZOCR3DAVCNFSM6AAAAABLGVDTQOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCOJYGUZDKOJYHA> . You are receiving this because you commented.Message ID: ***@***.***>

add tests

7b82283

gemtechd reviewed Jul 21, 2024

View reviewed changes

Fix comments and add mock

f84bdb9

gemtechd reviewed Jul 23, 2024

View reviewed changes

Fix comments other than mock

df6d004

gemtechd reviewed Jul 24, 2024

View reviewed changes

gemtechd added the ashdod label Jul 28, 2024

fix

4b3294f


		const getPointByXMock = jest
		.spyOn(Line.prototype, 'getPointByX')

		@@ -39,46 +45,70 @@ describe('Check calculateNOfLineFunction function', () => {
		})

		describe('Check getPointOnXAsis function', () => {

add tests #13

Are you sure you want to change the base?

add tests #13

Uh oh!

Conversation

lev527 commented Jul 21, 2024

Uh oh!

gemtechd left a comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gemtechd left a comment

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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemtechd left a comment

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment