-
Notifications
You must be signed in to change notification settings - Fork 41
test for geometry calculation #25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
I was not able to cover all the lines even though I did tests that went over those lines. |
gemtechd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have to change much more code in themodules
and use good mocks in the tests
modules/ecs6-class/line.js
Outdated
| class Line { | ||
| constructor({ point1 = new Point(), point2 = new Point(), n = undefined, slope = undefined }) { | ||
| constructor({ point1 = new Point(), point2 = new Point(), n = undefined, slope = undefined }) { | ||
| this.point1 = point1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you have to add more checks in the function
what happens when point is an array, what happens when n is a string etc...
| "test:coverage":"npm run test -- --coverage" | ||
| }, | ||
| "dependencies": { | ||
| "jest": "^29.7.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jest cannot be in the dependency section
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why didn't you change the code?
| describe('CALCULATE_N_OF_LINE_FUNCTION',() => { | ||
| it('should return the correct answer',() => { | ||
| const line = new Line({point1:new Point({x:6,y:4}),point2:new Point({x:2,y:2})}) | ||
| line.calculateSlope() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here you don't call this function, you test a different function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing the errors
and the coverage for this module is under 70
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't call functions inside the tests beside the function that you test
You have to see that other functions will be getting called inside the tested function
use mocks for a function that is being called inside an other one
|
I want to move on to the next task. |
gemtechd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the comments
modules/ecs6-class/line.js
Outdated
| this.point1 = point1; | ||
| this.point2 = point2; | ||
| this.slope = slope; | ||
| this.n = n; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if n is not undefined but is also not a number
if slope is not undefined but also not a number???
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i fixed it .
modules/geometry-calculation.js
Outdated
| throw new Error('the type of point is not Point') | ||
| const proxyLine = new Line({ point1: line.point1, point2: point }) | ||
| proxyLine.calculateSlope() | ||
| if (line.slope === proxyLine.slope) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Who said that the line's slope is already calculated?
| "test:coverage":"npm run test -- --coverage" | ||
| }, | ||
| "dependencies": { | ||
| "jest": "^29.7.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why didn't you change the code?
|
|
||
| "license": "ISC", | ||
| "devDependencies": { | ||
| "nyc": "^17.0.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this module?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I installed it when I didn't manage with the tests, but in the end I managed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where are the mocks??
|
Did you change code? |
gemtechd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Continue working on this task
| else{ | ||
| throw new Error('the type of point2 is not Point')} | ||
| else | ||
| throw new Error('the type of point2 is not Point') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Throw directly an error, like this, you don't mix up checks and codes
| // }) | ||
|
|
||
| // mockCalculatSlope.mockImplementation() | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove the remarks and console before pushing the commit
| it('An error should be thrown when line1 is not of type Line', () =>{ | ||
| const line2 = new Line({point1:new Point({x:8,y:3}),point2:new Point({x:6,y:1}),slope:1}) | ||
| line2.calculateNOfLineFunction() | ||
| line2.calculateNOfLineFunction() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't execute a different function in a unittest
| it('should return true when the point on this line',() => { | ||
| const line = new Line({point1:new Point({x:8,y:4}),point2:new Point({x:2,y:1}),slope:0.5}) | ||
| line.calculateNOfLineFunction() | ||
| line.calculateNOfLineFunction() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this line from the test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where are the mocks in the tests?
No description provided.