More Flexible Mocking in Unit Tests about focus-trap HOT 3 CLOSED

I could see that. You could still spy on activate() in its current form, though, provided you had access to the trap object returned by createFocusTrap(), couldn't you? That might be an "internal" that you'd have to give your test code access to, but otherwise, it wouldn't be much different from what you're proposing -- other than the fact that you'd like to be able to spy on a common function (used as a method) so that you don't have to expose your internal trap object.

I guess the constructor to a FocusTrap class would be something like constructor(state) since many of the functions inside createFocusTrap() operate on that shared object instead of the trap object itself.

Then we'd have to expose state as a property so that all those methods on the prototype would have access to it, which means we'd completely expose the internal state. Not the greatest thing. Right now, it's nicely hidden away.

We could use Object.defineProperty(this, 'activate', { enumerable: false, configurable: true, value() { ... } }) in the constructor, but then we'd be back to square one because "methods" would be tied to class instances, and that's basically what we have today with the plain object.

Not sure about this. I get your request, but it seems like it would lead to a complete refactor of the code. But maybe it's simpler than I'm thinking?

from focus-trap.

It is a bigger ask, yes, and I wasn't necessarily asking someone else to do it -- more creating a reminder.

The context being that in the Svelte world testing is more about the result, the testing library actively discourages testing component internals, and the library doesn't fully work in this environment (not demanding it should).

That said testing library's opinions aren't dear to my heart; if it's not something you like the idea of I respect that 🙇

from focus-trap.

I understand where you're coming from. I actually do like Testing Library's philosophy on testing as close to how the user will perceive it rather than testing internals.

That said, I'm not opposed to make it easier to test internals per se.

What I'm not excited about is a huge refactor of the code just to enable that. And I'm not excited about exposing all the guts of focus-trap to enable that either because people will take advantage of that in ways other than testing, and then it gets hairy.

I've written a Modal component in React that uses focus-trap-react and I use RTL to test it, and it works quite well, at least well enough to be confident about it.

There are tricks you can do as well to help test things, like you could add a "TEST_ENV" global var that Webpack/Rollup/Bundler knows to remove, but that is available for tests, and do something like set a class name on an element when your trap activates. Then you can write a test that checks the class name is there. (But we could debate that too, since you're technically altering the code to add this test-only functionality, etc., etc.)

Or maybe there is a way you could expose the trap instance, and then you could test all the internals you like on it.

My point is, I think that even with the way the code is today, there are a number of ways you could test what you're after without restoring to e2e testing in Cypress, which I agree is cumbersome and prefer to avoid myself.

from focus-trap.