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

feat: Add pretty assert #187

Merged
merged 5 commits into from Feb 15, 2019

Conversation

7 participants
@bokuweb
Copy link
Contributor

bokuweb commented Feb 10, 2019

Related
#179
bokuweb/deno-pretty-assert#3

@ry I created PR for now :)
If this PR will be approved, I'll archive original repo.

@CLAassistant

This comment has been minimized.

Copy link

CLAassistant commented Feb 10, 2019

CLA assistant check
All committers have signed the CLA.

@bokuweb bokuweb changed the title feat: Add pretty assert [WIP] feat: Add pretty assert Feb 10, 2019

@bartlomieju

This comment has been minimized.

Copy link
Contributor

bartlomieju commented Feb 10, 2019

Do you need to vendor pretty-format? It seems it contains it's own coloring logic and it makes me wonder if there won't be any conflicts with colors?

@bokuweb bokuweb force-pushed the bokuweb:add-pretty-assert branch from 04fe48a to bd4ecbc Feb 10, 2019

@bokuweb bokuweb force-pushed the bokuweb:add-pretty-assert branch from bd4ecbc to 92d870d Feb 10, 2019

@bokuweb bokuweb changed the title [WIP] feat: Add pretty assert feat: Add pretty assert Feb 10, 2019

@bokuweb

This comment has been minimized.

Copy link
Contributor Author

bokuweb commented Feb 10, 2019

@bartlomieju I used pretty-format to format object with no-highlight mode. This is because I would like to add diff color by myself :)

@bartlomieju

This comment has been minimized.

Copy link
Contributor

bartlomieju commented Feb 10, 2019

@bokuweb all clear, thanks. This module is very nice to have 👍

Show resolved Hide resolved testing/mod.ts Outdated
Show resolved Hide resolved testing/vendor/pretty-format.js Outdated

@bokuweb bokuweb changed the title feat: Add pretty assert [WIP] feat: Add pretty assert Feb 12, 2019

@bokuweb bokuweb force-pushed the bokuweb:add-pretty-assert branch from e2545c5 to 052609c Feb 15, 2019

@bokuweb bokuweb changed the title [WIP] feat: Add pretty assert feat: Add pretty assert Feb 15, 2019

@bokuweb bokuweb closed this Feb 15, 2019

@bokuweb

This comment has been minimized.

Copy link
Contributor Author

bokuweb commented Feb 15, 2019

@ry fixed. I ported pretty-format to testing/format.ts without unnecessary dependencies. Then I put this assertion to testing/pretty.ts.

BTW, are you interested in snapshot testing?? I thinks, it is useful in cases like this one. If you are interested in I would like to try to implement it.

@bokuweb bokuweb reopened this Feb 15, 2019

Show resolved Hide resolved testing/pretty.ts Outdated
Show resolved Hide resolved testing/diff.test.ts Outdated
@bartlomieju
Copy link
Contributor

bartlomieju left a comment

@bokuweb please use this license headers:

// Copyright 2018-2019 the Deno authors. All rights reserved. MIT license.

bokuweb added some commits Feb 15, 2019

@ry

This comment has been minimized.

Copy link
Contributor

ry commented Feb 15, 2019

@bokuweb This is great to have! thanks for the clean up

what's snapshot testing?

@ry

ry approved these changes Feb 15, 2019

Copy link
Contributor

ry left a comment

LGTM!

@ry ry merged commit ddafcc6 into denoland:master Feb 15, 2019

2 checks passed

denoland.deno_std #20190215.7 succeeded
Details
license/cla Contributor License Agreement is signed.
Details
@bokuweb

This comment has been minimized.

Copy link
Contributor Author

bokuweb commented Feb 15, 2019

@ry Thanks :) Could you please check following link about snapshot testing :)

https://jestjs.io/docs/en/snapshot-testing
https://github.com/avajs/ava/blob/master/docs/04-snapshot-testing.md

@bokuweb bokuweb deleted the bokuweb:add-pretty-assert branch Feb 15, 2019

case "removed":
return (s: string) => red(bold(s));
default:
return white;

This comment has been minimized.

@j-f1

j-f1 Mar 4, 2019

Should this be (s: string) => s since people with light terminal themes won’t be able to read white text?

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.