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

[expect] Allow creation of custom asymmetric matchers. #6649

Closed
conartist6 opened this issue Jul 6, 2018 · 24 comments
Closed

[expect] Allow creation of custom asymmetric matchers. #6649

conartist6 opened this issue Jul 6, 2018 · 24 comments

Comments

@conartist6
Copy link
Contributor

conartist6 commented Jul 6, 2018

🚀 Feature Proposal

This is a dog food feature request for the expect package.

Expect's internal matchers are defined using a powerful but private API: classes extending AsymmetricMatcher. While it is currently possible to import AsymmetricMatcher from expect's internal modules, it is not documented and expect.extend does not understand matchers defined as AsymmetricMatcher derivates. See this line where all non-internal matchers are assumed to be functions and wrapped in an AsymmetricMatcher.

I propose to document AsymmetricMatcher, give it an export symbol from the expect package, and modify expect.extend to wrap function type matchers in CustomMatcher (as is done already), while not performing any wrapping on matchers which extend AsymmetricMatcher.

Motivation

Providing complex named assertion types is done to facilitate the best possible messages for developers (and non developers!) when things break. By allowing matcher authors to create new asymmetric matchers they can better fulfill their goals, making Jest more powerful (without being any more complex) for end users.

Example

I am building assertions helpers for iterators. For example, I would like create
expect(iterable).toBeIterable()

It should pass if Symbol.iterator exists and is a function. not.ToBeIterable should pass only if Symbol.iterator does not exist.

Pitch

As proposed Jest API surface, this can't NOT be in the core platform.

@thymikee
Copy link
Collaborator

thymikee commented Jul 6, 2018

I think we had an issue for that, but I'm on mobile so it's hard to find

@conartist6
Copy link
Contributor Author

I searched all the open issues for "matcher" and didn't find anything in that list.

@conartist6
Copy link
Contributor Author

Found it: #4711.

@thymikee
Copy link
Collaborator

thymikee commented Jul 6, 2018

Can we close this then? ;)

@conartist6
Copy link
Contributor Author

Actually I can't tell why the other one was closed. They seem to have come to the same conclusion, that it would be necessary to expose and document the AsymmetricMatcher class, however that issue was closed after #5517 was merged. This confuses me, because that PR does not change the Jest API, it only adds a few more internal asymmetric matchers.

@rickhanlonii
Copy link
Member

We added custom asymmetric matchers in Jest 23, is this different?

@conartist6
Copy link
Contributor Author

conartist6 commented Jul 6, 2018

Huuuuh? Those are symmetric matchers in my mind. While their messages can be different when invoked in inverse, there is only one condition. The asymmetric matchers I am referring to have an inverse (.not) success condition which isn't a boolean not of their success condition. An existing example of this is expect.objectContaining.

@rickhanlonii
Copy link
Member

Yeah these are the same thing 😄

Check out the implementation here: https://github.com/facebook/jest/pull/5503/files#diff-033f219e5ac882702ea47d090593655fR57. Looks like we map the API of AsymmetricMatcher to the other matchers

Up for discussion on the limits/benefits of the API

@conartist6
Copy link
Contributor Author

conartist6 commented Jul 6, 2018

OK, I took some more time to see exactly what that feature is. I see now that it does allow asymmetry in matchers that are otherwise symmetric, but only with regard to matcher nesting.

This does, I think, solve my 80% use case, and I see how it solved the use cases presented in the other issue.

I still think that it makes sense to pursue this a bit further, because protocols have become a standard part of javascript life. In a protocol you use Symbols to attach data that fulfills a particular contract to an instance. For exmaple the Iterable protocol is satisfied when [Symbol.iterator] is a function which returns and iterator.

Symbols are the key to this pattern because they make intent unambiguous. It isn't possible that, say, you're accessing a data object that just happens to have a property named iterator defined. Thus when protocols are consumed, it isn't convention to guard them with checks like:

typeof obj[Symbol.iterator] === function && obj[Symbol.iterator]

While this is well and good, Symbol.iterator is a well known symbol, and as such it is expected to show up in code. Programmers can and will make mistakes, like assigning null to it. I would argue then that well written tests should guard against this kind of mistake.

The matcher that checks for proper protocol implementation, however, must be fundamentally asymmetric. If the symbol is found, the remaining conditions of the protocol must be fulfilled in order for the protocol to be satisfied and the code to be correct. If the assertion is that the protocol is not present, then the symbol must not be found.

I'm intrigued by the possibility, however, that what I'm suggesting is simply a new internal matcher along the lines of satisfiesProtocol.

@conartist6
Copy link
Contributor Author

conartist6 commented Jul 6, 2018

One possible version of satisfiesProtocol might look like:

expect.registerProtocol(Symbol.iterator, value => {
  const pass = typeof value === 'function';
  return { message: () => '...', pass }
}

expect.satistfiesProtocol(Symbol.iterator, myIterable);

@conartist6
Copy link
Contributor Author

conartist6 commented Jul 6, 2018

The argument against this is that there's no way to know whether a Symbol's value satisfies a particular contract. At most you'll be able to check the type of whatever's on there. To verify a Symbol.iterator method for real you'd have to try calling it and pass its result along to, say, Array.from. This is not something an expect matcher should be doing. It won't even work for protocols whose implementations are methods which take parameters. So trash that approach.

This suggests that when writing tests about protocols, you should simply check for a protocol's Symbol if you want to know if the protocol is present (which is symmetric). If you have implemented the protocol yourself, you should test your implementation.

@conartist6
Copy link
Contributor Author

conartist6 commented Jul 7, 2018

OK here's what I've got so far:

expect.extend({
  toBeIterable(received, argument) {
    let pass = Symbol.iterator in received;
    const iterator = received[Symbol.iterator];

    if (pass && argument && typeof argument.asymmetricMatch === 'function') {
      return {pass: argument.asymmetricMatch(iterator.call(received))};
    }

    return {
      message: () =>
        this.utils.matcherHint(`${pass ? '.not': ''}.toBeIterable`) +
        '\n\n' +
        `expected${pass ? ' not' : ''} to find a [Symbol.iterator] property, but Symbol.iterator was:\n` +
        `  ${this.utils.printReceived(received)}`,
      pass,
    };
  },

  yields(received, ...args) {
    let pass = args.reduce((pass, arg, i) => {
      return pass && arg === received.next().value;
    }, true)
    const done = received.next();
    pass = pass && done.done;
    return {
      message: () => `Didn't do the thing.`,
      pass,
    }
  },
})
function* iter() {
  yield 2;
  yield 4;
  yield 6;
}
expect(iter).toBeIterable(expect.yields(2, 4, 6))

This code works, (succeeds and fails in the correct situations), however the failure message of the inner expect is lost. I don't fully understand, from the perspective of Jest's design, why the message would be discarded. I'm presuming it has to do with matching the Jasmine API.

Note: this required a Jest API change already in order for argument.asymmetricMatch to receive the variadic arguments passed to yields.

@jayarjo
Copy link

jayarjo commented Dec 8, 2018

@conartist6 but can you access any utils via this in the yields matcher?

@jayarjo
Copy link

jayarjo commented Dec 8, 2018

I confirm that message from custom AssymetricMatcher is completely ignored. Also no utils are accessible via this and it probably should be reflected in the docs, otherwise it's confusing.

@jayarjo
Copy link

jayarjo commented Dec 8, 2018

Also one might assume that messages were meant to be taken into account, according to this illustration.

image

@SimenB
Copy link
Member

SimenB commented Dec 10, 2018

This looks more like a bug (error message not shown, if I understand correctly?) than a feature request - seeing as we do have expect.not.toBeMyCustomThing(). Mind opening up a new issue as a bug report?

@jayarjo
Copy link

jayarjo commented Dec 10, 2018

Here: #7492

@SimenB
Copy link
Member

SimenB commented Dec 10, 2018

Perfect, thank you!

@LukasBombach
Copy link

Hey guys, reanimating this old issue.

I came across this because I have this data structure (which I have already simplified for this issue):

type DiscoverEvent = [
  string,
  boolean,
  number,
  Advertisement,
];

interface Advertisement {
  localName?: string;
  txPowerLevel?: number;
  manufacturerData?: Buffer;
  serviceData?: Buffer;
}

So when I write my tests I want to see if my data matches my expected data:

const data = [
  "ae3f",
  true,
  4,
  {
    localName: "foobar"
  }
]

expect(data).toEqual(          
  expect.arrayContaining([      
    expect.any(String),
    expect.any(Boolean),
    expect.any(Number),
    expect.objectContaining({
      // difficult
    })
  ])
);

So the problem is that I'd like to use an asymmetric matcher that I can use within arrayContaining. As I understand it, I could not just use jest.extend because I could only write a custom matcher with that, like

expect(data).toBeAdvertisement();

which would work on its own, but I could not do this:

expect(data).toEqual(          
  expect.arrayContaining([      
    expect.any(String),
    expect.any(Boolean),
    expect.any(Number),
    expect(data[3]).toBeAdvertisement()
  ])
);

right? So to validate a data structure like this I would need a mechanism to write a matcher that can be used alongside with arrayContaining or objectContaining

@conartist6
Copy link
Contributor Author

I still really want this for iterables. To reiterate, I want it because when I write
a toBeIterableOf matcher I need two separate kinds of error messages:
"expected ... to contain items ... but instead got ..."
and:
"expected ... to be an iterable, but did no iterator was defined"

My use case is definitely not solved for yet.

@tonyhallett
Copy link

tonyhallett commented Oct 17, 2019

@LukasBombach

When you expect.extend asymmetric matchers get created for you although if the custom matcher is asynchronous it will not work. I am going to open an issue for this being mentioned in the documentation.

You can just supply an asymmetric matcher object as argument - see my example. ( Unlike one that is created for you it is not 'not' aware )

In case you were not aware the arrayContaining is not strict, it does not care about order.

Below is a custom matcher that works in your example.
The additional typing provides for a type safe expect object for working with your custom matchers and the custom asymmetric matchers. It only allows using non asynchronous custom matchers as asymmetric matchers. It also corrects definitely typed, preventing nonsense like expect(thing).not.not.not.not . The additional typing does break toMatchSnapshot and toMatchInlineSnapshot for property matchers but this will be corrected in the pull request to definitelytyped.

The difference between using a custom matcher as a custom matcher compared to an asymmetric matcher is that the this context is the matcher utils when used as a custom matcher.

describe('adding custom matchers with expect.extend creates Asymmetric Matchers',()=>{
    //this corrects definitely typed matcher returns but breaks 
    //toMatchSnapshot and toMatchInlineSnapshot for property matchers
    //a full change to definitely typed to follow
    type JestExpect=JestExpectProperties&{(actual: any):JestExpectations}
    //Typesafe custom matchers
    type RemoveFirstFromTuple<T extends any[]> = 
    T['length'] extends 0 ? [] :
        (((...b: T) => void) extends (a:any, ...b: infer I) => void ? I : [])
    interface AsymmetricMatcher {
        asymmetricMatch(other: unknown): boolean;
    }
    type CustomAsymmetricMatcher<T extends (...args: any) => any> = (...args: RemoveFirstFromTuple<Parameters<T>>) => AsymmetricMatcher;
    type CustomThrowingMatcher<T extends (...args: any) => any>=(...args: RemoveFirstFromTuple<Parameters<T>>)=>void
    type CustomResolveRejectMatcher<T extends (...args: any) => any>=(...args: RemoveFirstFromTuple<Parameters<T>>)=>Promise<void>
    type JestMatchers<R>=Omit<jest.Matchers<R>,'not'|'resolves'|'rejects'>
    type JestExpectations=
        JestMatchers<void>&
        {not:JestMatchers<void>}&
        {rejects:JestMatchers<Promise<void>>&{not:JestMatchers<Promise<void>>}}&
        {resolves:JestMatchers<Promise<void>>&{not:JestMatchers<Promise<void>>}}
    type JestExpectProperties={
        [K in keyof jest.Expect]:jest.Expect[K]
    }
    type NonAsyncMatchers<T extends jest.ExpectExtendMap>={
        [K in keyof T]:ReturnType<T[K]> extends Promise<jest.CustomMatcherResult>? never:K
    }[keyof T]
    
    //as mentioned above breaks toMatchSnapshot and toMatchInlineSnapshot for property matchers
    type JestExpectAndCustomMatchers<T extends jest.ExpectExtendMap>=
        JestExpectProperties&

        ((actual: any)=>
            JestExpectations&
            {[K in keyof T]:CustomThrowingMatcher<T[K]>}&
            {not:{[K in keyof T]:CustomThrowingMatcher<T[K]>}}&

            {rejects:{[K in keyof T]:CustomResolveRejectMatcher<T[K]>}&{not:{[K in keyof T]:CustomResolveRejectMatcher<T[K]>}}}&
            {resolves:{[K in keyof T]:CustomResolveRejectMatcher<T[K]>}&{not:{[K in keyof T]:CustomResolveRejectMatcher<T[K]>}}}
        )
        
        &
        {[K in NonAsyncMatchers<T>]:CustomAsymmetricMatcher<T[K]>}&
        {
            not:jest.Expect["not"]&{[K in NonAsyncMatchers<T>]:CustomAsymmetricMatcher<T[K]>}
        }

    function expectExtend<T extends jest.ExpectExtendMap> (matchers:T):JestExpectAndCustomMatchers<T>
    {
        expect.extend(matchers);
        return expect as any;
    }

    it('should help LukasBombach',()=>{
        interface Advertisement {
            localName?: string;
            txPowerLevel?: number;
            manufacturerData?: Buffer;
            serviceData?: Buffer;
        }

        const isMatching=(received:any,match:(received:any)=>jest.CustomMatcherResult | Promise<jest.CustomMatcherResult>)=>{
            return match(received);
        }
        const extendedExpect=expectExtend({isMatching});

        function objectHasProperty<T extends Object>(object:T,properties:Array<keyof T>){
            let pass=false;
            for(let i=0;i<properties.length;i++){
                pass=!!object[properties[i]];
                if(pass) break;
            }
            return pass;
        }
        function checkIsAdvertisement(value:any){
            return objectHasProperty<Advertisement>(value,["localName","manufacturerData","serviceData","txPowerLevel"])
        }
        
        function expectData(data:any,match:boolean){
            function isAdvertisement(){
                return extendedExpect.isMatching((entry)=>{
                    let pass=checkIsAdvertisement(entry);
                    return {
                        pass,
                        message:''
                    }
                })
            }
            let matchers=expect(data);
            if(!match) matchers = matchers.not;

            matchers.toEqual(          
                expect.arrayContaining([      
                  expect.any(String),
                  expect.any(Boolean),
                  expect.any(Number),
                  
                  isAdvertisement()
                ])
              );
        }
        
        expectData([
            "ae3f",
            true,
            4,
            {
                localName: "foobar"
            }
        ],true);

        //just to demo arrayContaining
        expectData([
            {
                txPowerLevel: 42
            },
            "ae3f",
            true,
            4,
            
        ],true);
        
        //different property
        expectData([
            "ae3f",
            true,
            4,
            {
                else: undefined
            }
        ],false)
        
        //missing entry
        const missingEntry=[
            "ae3f",
            true,
            4
        ]
        interface IAsymmetricMatcher{
            asymmetricMatch(value: unknown):boolean
        }
        expectData(missingEntry,false);
          
        //manually providing asymmetric matcher
        const isAdvertismentMatcher:IAsymmetricMatcher={
            asymmetricMatch:function(value){
                return checkIsAdvertisement(value);
            }
        }
        expect(missingEntry).not.toEqual(          
            expect.arrayContaining([      
              expect.any(String),
              expect.any(Boolean),
              expect.any(Number),
              
              isAdvertismentMatcher
            ])
          );
    })
})

@LukasBombach
Copy link

Oh wow, what a detailed answer, thank you so much @tonyhallett

@tonyhallett
Copy link

@LukasBombach no problem although I was incorrect with part of my custom typing. I will revise.

@github-actions
Copy link

This issue 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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants