Pretty-print asymmetric matchers #2476

thymikee · 2016-12-30T14:41:14Z

Summary

Provide a basic proof of concept
Extract asymmetric-matchers: any, anything, arrayContaining, objectContaining, stringMatching from jasmine-utils into separate module
Rename <jasmine.asymmetricMatcherName> to just <asymmetricMatcherName>
Refactor asymmetric-matchers
Improve Flow coverage
Create AsymmetricMatchers plugin for pretty-format
Use the plugin instead of basic proof of concept
Tests

codecov-io · 2016-12-30T15:01:34Z

Current coverage is 67.43% (diff: 97.40%)

Merging #2476 into master will increase coverage by 2.60%

@@             master      #2476   diff @@
==========================================
  Files           125        125          
  Lines          4871       4889    +18   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           3158       3297   +139   
+ Misses         1713       1592   -121   
  Partials          0          0

Powered by Codecov. Last update fa39ef8...fd7a7ec

thymikee · 2017-01-02T21:14:32Z

Added tests for asymmetric-matchers, as they are our responsibility now.

jest-bot · 2017-01-10T02:54:22Z

	Warnings
⚠️	Please ensure that `@flow` is enabled on: packages/jest-matchers/src/__tests__/asymmetric-matchers-test.js and packages/pretty-format/src/__tests__/AsymmetricMatcher-test.js

Generated by 🚫 dangerJS

thymikee · 2017-01-12T11:50:29Z

Here's a preview of how it looks.

cpojer · 2017-01-12T13:23:51Z

packages/jest-matchers/src/asymmetric-matchers.js

+
+  asymmetricMatch(other: Array<any>) {
+    const className = Object.prototype.toString.call(this.sample);
+    if (className !== '[object Array]') {


Can we do Array.isArray.

cpojer · 2017-01-12T13:35:56Z

packages/jest-matchers/src/jasmine-utils.js

@@ -276,102 +258,12 @@ function fnNameFor(func) {
    return func.name;
  }

-  var matches = func.toString().match(/^\s*function\s*(\w*)\s*\(/);
+  const matches = func.toString().match(/^\s*function\s*(\w*)\s*\(/);
  return matches ? matches[1] : '<anonymous>';


did you mean to the fnNameFor function here? It's in the other file too.

oh, oops, nevermind.

cpojer · 2017-01-12T13:40:04Z

packages/pretty-format/src/__tests__/AsymmetricMatcher-test.js

+
+const prettyFormat = require('../');
+const AsymmetricMatcher = require('../plugins/AsymmetricMatcher');
+const jestExpect = require('jest-matchers');


not really sure why this works. You need to make jest-matchers a devDependency of pretty-format.

BTW, do I really need jestExpect here instead of available expect? I mean this is test for pretty-format, not for a matcher...

Try it! If it works, that should be fine here.

cpojer · 2017-01-12T13:43:15Z

I'm a bit unsure about the printing here. Could we simplify it and get rid of what Jasmine does? How about:

ArrayContaining [

]

ObjectContaining {

}

and similar. What do you think?

cpojer · 2017-01-12T13:43:47Z

Overall this looks really great!

thymikee · 2017-01-12T13:44:15Z

yes!!!!! ❤️
This should also look good on diffs

thymikee · 2017-01-12T13:44:44Z

I'll iterate over the PR sometime today

cpojer · 2017-01-15T14:54:05Z

packages/jest-matchers/src/asymmetric-matchers.js

+  }
+
+  toAsymmetricMatcher(print: Function) {
+    return 'ArrayContaining ' + print(this.sample).replace(/Array\s+/, '');


Won't this get rid of all sub arrays? I'm a bit worried this isn't going to be stable and break in subtle ways and when I made the suggestion, I didn't know how to work around this. Another idea would be to say:

Contains Array [ 1, 2, ]

that way you only need to prepend parts of the string.

The same goes for all the other .replace calls in this file. Ideally I'd like to find a way without this.

This is safer. But with this regex I don't pass g flag, so it only replaces first occurrence. Tested this on real examples, but I could add an extra test for that

I think we print the name of constructors, so we could theoretically do something like this:

class ArrayContaining extends Array {} const array = new ArrayContaining(); array.push(...this.sample); return array; // pretty format this.

that may work! In ES2015+ you can subclass and extend arrays.

cpojer · 2017-01-15T14:57:09Z

Alright, I think if we can improve how we print this, this is ready to go.

thymikee · 2017-01-15T17:27:59Z

Rendering of ArrayContaining, ObjectContaining and StringMatching is now moved to pretty-format plugin, without "regexping" anything.

cpojer · 2017-01-15T21:12:31Z

Stellar work! Thanks for sticking with me through the iterations.

* First attempt to fix this * Extract asymmetric-matchers from jasmine-utils * Remove <jasmine.*> adnotations * Remove obsolete snapshots * Add AsymmetricMatcher plugin for pretty-format * Refactor asymmetric-matchers * Use AsymmetricMatcher plugin in diffs * Remove unused prettyFormat from asymmetric-matcher * Check for object presence in AsymmetricMatcher * Update snapshots * Tests for asymmetric-matchers * Update snapshots with patch marks * Adjust printing of asymmetric matchers * Use class instances instead of regex to render values

github-actions · 2021-05-14T02:30:46Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

facebook-github-bot added the CLA Signed ✔️ label Dec 30, 2016

thymikee changed the title ~~[WIP] Pretty-print asymmetric matchers~~ Pretty-print asymmetric matchers Jan 2, 2017

thymikee requested a review from cpojer January 2, 2017 17:40

thymikee force-pushed the fix/asymmetric-matchers branch from 4cd3f63 to 0a27cda Compare January 2, 2017 17:53

thymikee added 12 commits January 9, 2017 23:04

First attempt to fix this

14f5638

Extract asymmetric-matchers from jasmine-utils

9b538b9

Remove <jasmine.*> adnotations

c7a2bff

Remove obsolete snapshots

1da977e

Refactor asymmetric-matchers

f4d5d9c

Add AsymmetricMatcher plugin for pretty-format

52787d8

Use AsymmetricMatcher plugin in diffs

8555959

Remove unused prettyFormat from asymmetric-matcher

0eefe65

Check for object presence in AsymmetricMatcher

5d6f6b4

Update snapshots

560d15c

Tests for asymmetric-matchers

d741f29

Update snapshots with patch marks

538fb18

thymikee force-pushed the fix/asymmetric-matchers branch from b8cf138 to 538fb18 Compare January 9, 2017 22:19

thymikee mentioned this pull request Jan 10, 2017

Partial matching for mock assertions #2137

Closed

cpojer reviewed Jan 12, 2017

View reviewed changes

Adjust printing of asymmetric matchers

9692e18

cpojer reviewed Jan 15, 2017

View reviewed changes

Use class instances instead of regex to render values

fd7a7ec

cpojer merged commit 07b4bf7 into jestjs:master Jan 15, 2017

thymikee deleted the fix/asymmetric-matchers branch January 15, 2017 21:39

github-actions bot locked as resolved and limited conversation to collaborators May 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pretty-print asymmetric matchers #2476

Pretty-print asymmetric matchers #2476

thymikee commented Dec 30, 2016 •

edited

Loading

codecov-io commented Dec 30, 2016 •

edited

Loading

thymikee commented Jan 2, 2017

jest-bot commented Jan 10, 2017 •

edited

Loading

thymikee commented Jan 12, 2017

cpojer Jan 12, 2017

cpojer Jan 12, 2017

cpojer Jan 12, 2017

cpojer Jan 12, 2017

thymikee Jan 12, 2017

cpojer Jan 12, 2017

cpojer commented Jan 12, 2017

cpojer commented Jan 12, 2017

thymikee commented Jan 12, 2017

thymikee commented Jan 12, 2017

cpojer Jan 15, 2017

cpojer Jan 15, 2017

thymikee Jan 15, 2017

cpojer Jan 15, 2017

cpojer commented Jan 15, 2017

thymikee commented Jan 15, 2017

cpojer commented Jan 15, 2017

github-actions bot commented May 14, 2021

Pretty-print asymmetric matchers #2476

Pretty-print asymmetric matchers #2476

Conversation

thymikee commented Dec 30, 2016 • edited Loading

codecov-io commented Dec 30, 2016 • edited Loading

Current coverage is 67.43% (diff: 97.40%)

thymikee commented Jan 2, 2017

jest-bot commented Jan 10, 2017 • edited Loading

thymikee commented Jan 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpojer commented Jan 12, 2017

cpojer commented Jan 12, 2017

thymikee commented Jan 12, 2017

thymikee commented Jan 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpojer commented Jan 15, 2017

thymikee commented Jan 15, 2017

cpojer commented Jan 15, 2017

github-actions bot commented May 14, 2021

thymikee commented Dec 30, 2016 •

edited

Loading

codecov-io commented Dec 30, 2016 •

edited

Loading

jest-bot commented Jan 10, 2017 •

edited

Loading