Added PollingControllerOnly #1873
Conversation
| _executePoll = createExecutePollMock(); | ||
| } | ||
| const c = new MyClass(); | ||
| expect(c).toBeDefined(); |
There was a problem hiding this comment.
Hmm. It seems like you could have written this test without extending from anything and it would have still passed.
Why is a class that extends from PollingControllerBase any different from a class that extends from PollingController or PollingControllerV1? Can we copy over the existing tests that we have? It seems like we'd still want to verify that such a class that extends from PollingControllerBase can be used to poll in all the ways it's supposed to.
There was a problem hiding this comment.
I'm not really looking to test that the functionality works again, but that my PollingControllerMixin(Empty) line works properly, that it can extend this empty class and still have all the methods.
I updated the test to check all the methods exist.
There was a problem hiding this comment.
I'm not really looking to test that the functionality works again,
Sorry, I don't understand this decision. This interface is just as valid as the other interfaces you have here, and therefore the functionality it has is just as important to test, in exactly the same way. It seems to me that you think that this class ought to work because it's based on the same class as the other two classes. It's true they all share the same implementation, but that implementation is completely private to consumers.
…od checks to tests
Co-authored-by: jiexi <[email protected]>
Explanation
Added PollingControllerOnly to extend from an empty class. This will allow classes that previously are just classes that don't extend from BaseV2 or V2 to extend from this new PollingControllerOnly.
Changelog
@metamask/polling-controller