close
The Wayback Machine - https://web.archive.org/web/20210119020715/https://github.com/bfirsh/jsnes-web/issues/165
Skip to content

/ jsnes-web

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

Jump to bottom

Add tests for gamepad support #165

Open
bfirsh opened this issue Jan 10, 2019 · 10 comments
Open

Add tests for gamepad support #165

bfirsh opened this issue Jan 10, 2019 · 10 comments
Labels
good first issue help wanted

Comments

@bfirsh
Copy link
Owner

@bfirsh bfirsh commented Jan 10, 2019 •
edited

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.
@bfirsh bfirsh mentioned this issue Jan 10, 2019
Merged
@tario
Copy link
Contributor

@tario tario commented Jan 11, 2019 •
edited

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 ?
@bfirsh
Copy link
Owner Author

@bfirsh bfirsh commented Jan 11, 2019

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. :)
@tario
Copy link
Contributor

@tario tario commented Jan 11, 2019

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)
@bfirsh
Copy link
Owner Author

@bfirsh bfirsh commented Jan 11, 2019

No worries! There are a couple really simple ones - the files ending in .test.js in src/.
@tario
Copy link
Contributor

@tario tario commented Jan 11, 2019

Great, I will follow the actual scheme then
@bfirsh
Copy link
Owner Author

@bfirsh bfirsh commented Jan 14, 2019

@tario Seems like the gamepad support is breaking non-webkit browsers too. This error is coming from IE: https://sentry.io/share/issue/b0c3a6b734294b30b6414c548d57a410/
@tario
Copy link
Contributor

@tario tario commented Jan 15, 2019

I am sorry to hear that, maybe I should fix that in a separated pull-request. What platforms we should cover ? IE? edge too?
@tario
Copy link
Contributor

@tario tario commented Jan 15, 2019

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 ?
@tario
Copy link
Contributor

@tario tario commented Jan 15, 2019

Anyway. I will fix the case where both getGamepads and webkitGetGamepads are not present. I think I have to code more defensively, sorry
@tario
Copy link
Contributor

@tario tario commented Jan 15, 2019

Here it is #167
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue help wanted
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
@bfirsh @tario
You can’t perform that action at this time.