Ignoring function pointers in preact-jsx-chai unit tests

[olegded]

I would like to ignore function pointers for any bound function in some preact component in unit tests. Right now, you have to mock them.

Thanks to @developit for help how to do it!

[developit]

Looking for feedback from the community on this one! /cc @INRIX-mark-guinn @impronunciable @legomushroom @c2h5oh @Robert-Wett (guessing as to who is using this haha)

[legomushroom]

@olegded could you make a simple code sample on how you've been doing that?

[INRIX-mark-guinn]

Yeah, I'd need to see an example. I'm not clear on what you're trying to accomplish.

[legomushroom]

@INRIX-mark-guinn as I've got it the issue is about function bindings on components, like onClick={ someHandler }. So you need to mock up the someHandler function to test the component. @olegded am I right?

[developit]

I had sent @olegded this previously - it's what we're doing right now. In my head though, it's basically just mocking out a function's name and whether it was bound or not. This makes me wonder if asserting the sameness of name+bound is even useful.

// create a function named onClick:
function onClick() {}   // [Function onClick]

// if you are using bind(), match it:
let bound = onClick.bind(null);   // [Function bound onClick]

// example:
expect(
  <div onClick={bound} />
).to.eql(
  <div onClick={bound} />
);

[legomushroom]

@developit I think, by default, I don't want the bound functions to be strictly the same, so I want to test if the markup is ok and some handler function is in there. But in some cases, I do want to test if there is actual handler is strictly the same as I use in my component, so having this option should be very useful.

[developit]

I'm not sure if strict equality for the handlers is possible. Also, we bind via two mechanisms: .bind() in render() and property initializer method shorthand. The issue I've run into is that there is actually no way to access those methods in an assertion, because they are created/bound during render (which happens within the assertion itself). For unbound methods (class methods) I could see a way to pass those to the assertion, but I can't think of a solution for bound/instance methods.

class Foo {
  bar = () => {}
  foo() {}
  render() {
    return <div onFoo={this.foo.bind(this)} onBar={this.bar} />
  }
}

Foo.bar // undefined
Foo.prototype.bar // undefined
Foo.prototype.foo  // function foo() {}

expect(<Foo />).to.eql(
  <div onFoo={Foo.prototype.bar ***1} onBar={Foo.prototype.foo.bind(***2)} />
);

// ***1: doesn't exist so we can't assert on it
// ***2: no instance reference at this point to bind to

Hmm.

[legomushroom]

So you can't even use a decorator to bound the methods, right? Thinking maybe some extra utility to extract the functions before run time in expect and suppress the functions in eql with them might work? Can get messy tho I guess.

[developit]

Correct - there's no way to assert the parity between the decorated and undecorated method, sadly. One possibility I could think of would be to use two components?

class Foo {
  bar = () => {}
  render() {
    return <div onClick={this.bar.bind(this)} />
  }
}

let foo;  // holds our Foo instance hopefully?
const Expected = () => (
  <div onClick={foo.bar.bind(foo)} />
);

expect(<Foo ref={ c => foo = c } />).to.eql(<Expected />);

Honestly I'm not even sure this is supported in preact-render-to-string though - refs don't really do anything without mounting.

[olegded]

@legomushroom

I had some troubles to test some preact component against expected markup.

Here is a simplified example which is now working due to help by @developit

export class SomeClass extends preact.Component {
    onClick(event) {
        ...
    }

    render() {
        <div class="someClass">
            ...
            <input type="radio" name="someName"
                ...
                onClick={::this.onClick}
                ...
            </input>
            ...
        </div>
    }
}

Now, the following unit test

it('description', function() {
    ...
    expect(
        <SomeClass/>
    ).to.eql(
        // expected HTML markup
    );
    ...
});

was failing due to different function pointers:

    - onClick={[Function bound onClick]}
    + onClick={[Function onClick]} // mocked function

Current solution is the following one:

it('description', function() {
    let onClick = function(event) {};

    ...
    expect(
        <SomeClass/>
    ).to.eql(
        ...
        onClick={onClick.bind(null)}
        ...
    );
    ...
});

By looking at the jsx-chai documentation, I found the following note about function pointers

to.deep.equal and to.eql will not check for function references,
it only checks that if a function was expected somewhere,
there's also a function in the actual data.

It's your responsibility to then unit test those functions.

and was asking myself how I can do it using the preact-jsx-chai library.

[developit]

That might be a happy medium - just assert that a function was passed as a prop, not that it has a given name or binding.

[legomushroom]

Nice! So you have that option to loosely check the function presence. It compares functions by their string representation anyways, right? (e.g. by whatever returned from the toString method of the function)

[developit]

Right now it's just using the default behavior of pretty-format, which appears to just match Object.prototype.toString.call(fn). It doesn't stringify the body of the function, just the function's definition. I think just asserting that any function was provided is a nice compromise. What say you all? :)

[developit]

So I've just published an update (2.2.0) that provides options for turning of function prop comparison entirely, or just turning off function name/binding comparison (just a type check).

Think we need more here? Are the defaults fine staying as functions compared by name+bound status?

[developit]

Closing for now since the option now exists, but I would love feedback from you guys on whether the default currently makes sense!

[olegded]

@developit Thanks! We will try to test it as far as possible.

[olegded]

@developit assertJsx.options.functions = false option works for us. I think the default makes sence, at least from our point of view.

Thanks one more time!