Skip to content

add tests#24

Open
M-Grun wants to merge 3 commits into
gemtechd:mainfrom
M-Grun:tests
Open

add tests#24
M-Grun wants to merge 3 commits into
gemtechd:mainfrom
M-Grun:tests

Conversation

@M-Grun

@M-Grun M-Grun commented Jul 23, 2024

Copy link
Copy Markdown

No description provided.

@gemtechd gemtechd left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

See the comments

Comment thread modules/ecs6-class/line.js Outdated
class Line {
constructor({ point1 = new Point(), point2 = new Point(), n = undefined, slope = undefined }) {
if(!(point1 instanceof(Point)) || !(point2 instanceof(Point)))
throw new Error('argument should be a point')

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Which argument?

Comment thread modules/ecs6-class/line.js
Comment thread modules/ecs6-class/line.js
Comment thread modules/ecs6-class/point.js
if(!(line1 instanceof(Line)) || !(line2 instanceof(Line)))
throw new Error('the argument should be line')
if (line1.slope === line2.slope) {
if (line1.n === line2.n) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

What happens if the slope was not calculated or n was not calculated?

Comment thread package.json
"coverage": "npm run test -- --coverage"
},
"dependencies": {
"jest": "^29.7.0"

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

this is not the right place for the jest module

Comment thread tests/ecs6-class/line.test.js Outdated
const line1 = new Line({});

describe('ERRORS', () => {
test('should throw string error when argument are no point', () => {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Where are the good result for the line?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Where are the good tests for the constructor?

Comment thread tests/ecs6-class/line.test.js Outdated
})

describe('CALCULATE_SLOPE', () => {
test('should calculate the slope', () => {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Where are the excpetions of the function?

const LineInstance = new Line({});
LineInstance.getPointByY = mockGetPointByY;
const result = LineInstance.getPointOnXAsis();
expect(mockGetPointByY).toHaveBeenCalledWith(0);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

this expect line is unnecessary

const point6 = new Point({ x: 1, y: 1 });
const line4 = new Line({ point1: point5, point2: point6, slope: 1, n: 0 });
const line5 = new Line({ point1: point5, point2: point4, slope: 1, n: 3 });

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Where is the mocks for the line class?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

don't create instances of objects in the head of the module, each test gets its own objects to test

@gemtechd gemtechd left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

You have to build your tests structure again.
use the it test function instead of test

Comment thread package.json Outdated
"dependencies": {
"jest": "^29.7.0"
"jest": "^29.7.0",
"coverage": "npm run test -- --coverage"

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

is coverage a dependency?
does jest belong to the dependencies?

Comment thread tests/ecs6-class/line.test.js Outdated
const line3 = new Line({});
line3.slope = 2;
const line4 = new Line({ point1, point2 });
line4.n = 0;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Don't define some objects in the begin of the module, each test should get its own objects,

Comment thread tests/ecs6-class/line.test.js Outdated
const line1 = new Line({});

describe('ERRORS', () => {
test('should throw string error when argument are no point', () => {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Where are the good tests for the constructor?

LineInstance.getPointByX = mockGetPointByX;
const result = LineInstance.getPointOnYAsis();
LineInstance.getPointOnYAsis();
expect(mockGetPointByX).toHaveBeenCalledWith(0);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

What is this test?

const point6 = new Point({ x: 1, y: 1 });
const line4 = new Line({ point1: point5, point2: point6, slope: 1, n: 0 });
const line5 = new Line({ point1: point5, point2: point4, slope: 1, n: 3 });

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

don't create instances of objects in the head of the module, each test gets its own objects to test

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants