-
Notifications
You must be signed in to change notification settings - Fork 41
tests and mocks #16
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?
tests and mocks #16
Conversation
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.
change your code according to the comments
and write real es6 class mocks
| this.slope = slope; | ||
| this.n = n; | ||
| if ((this.slope !== undefined && typeof (this.slope) !== 'number') || (this.n !== undefined && typeof (this.n) !== 'number')) | ||
| throw Error('this value is 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.
which parameter is not a number?
| this.y = y; | ||
| constructor({ x = 0, y = 0 } = {}) { | ||
| if (typeof (x) !== "number" || typeof (y) !== "number") | ||
| throw Error("this value is 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.
who is not a number, the x or the y?
| return distance; | ||
| } | ||
| else | ||
| throw Error("this point is not a 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.
which parameter is not a point
and it will be good if I would be noticed if a parameter in undefined
| if (line1.n === line2.n) { | ||
| return true | ||
| if (line1 instanceof Line && line2 instanceof Line) { | ||
| if (line1.slope === line2.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.
Is the slope already calculated?
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 didn't realize the slope is already calculated
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.
It doesn't have to be
it can be undefined
| return junctionPoint | ||
| } | ||
| else | ||
| throw Error("this line is not a Line") |
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.
which line is not a line?
|
|
||
| describe('Line', () => { | ||
| const mockConstructorLine = jest.fn(constructor); | ||
| const mockConstructorPoint = jest.fn(constructor); |
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.
this is not mocking a class only a constructor function
| expect(() => new Line({ slope: { t: 5 } })).toThrow('this value is not a number') | ||
| expect(() => new Line({ n: "vbg" })).toThrow('this value is not a number') | ||
| expect(() => new Line({ n: [8] })).toThrow('this value is not a number') | ||
| expect(() => new Line({ n: { i: 5 } })).toThrow('this value is 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.
change your errors in the code, and change the tests
| point.moveVertical(5); | ||
| expect(point.y).toBe(10); | ||
| point.moveVertical(-3); | ||
| expect(point.y).toBe(7); |
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 the AAA methodology?
work according it
| describe('geometry-calculation', () => { | ||
| const mockConstructorLine = jest.fn(constructor) | ||
| const mockConstructorPoint = jest.fn(constructor) | ||
|
|
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.
this is not how to mock a class, only a constructor.
you could have written only one mockFunction for the constructor,
it could have been used for all constructors: line, point and more
write real mocks for a class
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.
Is this a correct direction for mock, or does it not even come close?
const mockPoint = {
x: 0,
y: 0,
moveVertical: sinon.stub().callsFake((value) => {
console.log(value);
return mockPoint.y + value;
}),
moveHorizontal: sinon.stub().callsFake((value) => {
return mockPoint.x + value;
})
};
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 use the sinon object to mock
use jest.mock('module', ()=>{})
there are a few ways
search build mock tests with jest, mock es6 class with jest, mock partial class es6 jest, __esmodule
No description provided.