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

Assertion improvement #267

Open
wants to merge 10 commits into
base: master
from

Conversation

4 participants
@zekth
Copy link
Contributor

zekth commented Mar 12, 2019

Following the refactor about AssertionError : #249
Here is a refactor of the message building using the pretty module of testing. Change the naming from left/right to actual/expected to fit with the naming of the rest of the API.

Also now there is no more circular references between asserts / pretty.

I may need your opinion on the format of the prompt. ATM it's readable enougth for me. Your thoughts?

cc @bartlomieju @ry

Example of prompt:
2019-03-12 15_01_04-MINGW64__c_projects_deno_std

@CLAassistant

This comment has been minimized.

Copy link

CLAassistant commented Mar 12, 2019

CLA assistant check
All committers have signed the CLA.

super(message);
this.name = "AssertionError";
this.name = "Assertion Error";

This comment has been minimized.

@ry

ry Mar 12, 2019

Contributor

Why? I think the name of an error should be the same as its class name?

This comment has been minimized.

@kitsonk

kitsonk Mar 13, 2019

Contributor

I agree... It should be AssertionError

> new RangeError().name
"RangeError"
> new TypeError().name
"TypeError"

This comment has been minimized.

@zekth

zekth Mar 13, 2019

Author Contributor

Seems legit.

@zekth zekth marked this pull request as ready for review Mar 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.