-
Notifications
You must be signed in to change notification settings - Fork 162
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
Add tests for gamepad support #165
Comments
Ok! I will work on that. I will start by writing tests for GamepadController class (mocking gamepad API) . We need tests for react components that were modified too ? |
Awesome thank you! Testing the integration of the modal with the controllers is probably a good idea (I don’t think there is any testing for the custom keyboard commands) but a test for the gamepadcontroller would also be useful. :) |
I can't see tests on current version of jsnes-web So, I will follow this guide https://facebook.github.io/create-react-app/docs/running-tests to start adding the tests. It is ok ? (sorry, I am newbie on react) |
No worries! There are a couple really simple ones - the files ending in |
Great, I will follow the actual scheme then |
@tario Seems like the gamepad support is breaking non-webkit browsers too. This error is coming from IE: https://sentry.io/share/issue/b0c3a6b734294b30b6414c548d57a410/ |
I am sorry to hear that, maybe I should fix that in a separated pull-request. What platforms we should cover ? IE? edge too? |
Just tested jsnes.org on edge and IE11 and the gamepad seems to be broken at all. Also tested on https://html5gamepad.com/ and again the gamepad doesn't seems to work at all. Gamepad API should be working on these browsers ? |
Anyway. I will fix the case where both getGamepads and webkitGetGamepads are not present. I think I have to code more defensively, sorry |
Here it is #167 |
There should be some basic automated tests for the gamepad support added in #164. I don't have a gamepad, and I imagine most contributors won't, so it'll be very easy to cause regressions. It is also impossible for me or other contributors to refactor/touch that code because we can't test it.
Once there are some tests, there are some refactorings I have noted in #164 that are probably worth doing.
The text was updated successfully, but these errors were encountered: