Skip to content
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

[TEST] Update error message in e2e tests #689

Merged
merged 4 commits into from
Oct 2, 2020

Conversation

csouchet
Copy link
Member

@csouchet csouchet commented Oct 1, 2020

Add custom message when a cell is not defined

Closes #672

@csouchet csouchet added the refactoring Code refactoring label Oct 1, 2020
@csouchet csouchet force-pushed the 672-Update_error_message_in_e2e_tests branch from 766171d to 58f70c7 Compare October 1, 2020 16:01
@csouchet csouchet marked this pull request as ready for review October 1, 2020 16:07
@csouchet csouchet requested review from tbouffard and aibcmars October 1, 2020 16:07
@tbouffard tbouffard modified the milestone: 0.4.0 Oct 2, 2020
const cell = bpmnVisualization.graph.model.getCell(cellId);
if (cell) {
return {
message: () => `Expected cell with id '${cellId}' NOT to be defined`,
Copy link
Member

@tbouffard tbouffard Oct 2, 2020

Choose a reason for hiding this comment

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

❓ if this more accurate, I guess that when the expect pass, we have to provide what is ok

Suggested change
message: () => `Expected cell with id '${cellId}' NOT to be defined`,
message: () => `Expected cell with id '${cellId}' exists in the mxGraph model`,

Copy link
Member Author

@csouchet csouchet Oct 2, 2020

Choose a reason for hiding this comment

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

The message must be negative. See the Jest exemple: https://jestjs.io/docs/en/expect#expectextendmatchers
I run the test with a error, and the right message is displayed.

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough, I hadn't found the Jest expect convention.
The Jest doc is clear about that and the new messages are fine.

};
} else {
return {
message: () => `Expected cell with id '${cellId}' to be defined`,
Copy link
Member

@tbouffard tbouffard Oct 2, 2020

Choose a reason for hiding this comment

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

Suggested change
message: () => `Expected cell with id '${cellId}' to be defined`,
message: () => `Expected cell with id '${cellId}' not found in the mxGraph model`,

Copy link
Member Author

Choose a reason for hiding this comment

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

The message must be positive.

Copy link
Member

@tbouffard tbouffard left a comment

Choose a reason for hiding this comment

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

Work as expected, minor changes in the expect messages could be done

@csouchet
Copy link
Member Author

csouchet commented Oct 2, 2020

Work as expected, minor changes in the expect messages could be done

The messages are updated. But, the expectations are good as they are.

@csouchet csouchet force-pushed the 672-Update_error_message_in_e2e_tests branch from ccd7291 to 8220ad6 Compare October 2, 2020 12:09
@csouchet csouchet requested a review from tbouffard October 2, 2020 12:12
Copy link
Member

@tbouffard tbouffard left a comment

Choose a reason for hiding this comment

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

Ok, with the new messages

@csouchet csouchet merged commit 0d4b86b into master Oct 2, 2020
@csouchet csouchet deleted the 672-Update_error_message_in_e2e_tests branch October 2, 2020 12:19
@csouchet csouchet added the hacktoberfest-accepted Accepted Pull Request during Hacktoberfest label Oct 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Accepted Pull Request during Hacktoberfest refactoring Code refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TEST] Update error message in e2e tests
2 participants