You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Showdown has issues with buggy move mechanics being found in production. Ideally, we would have tests for moves to catch these before this happens. I know we can't feasibly write comprehensive tests to prevent all of these now, as there are just too many moves. However, we can mitigate this, and one simple idea is to require anyone who makes a PR fixing a certain mechanic to also make unit tests that demonstrate the mechanic has been fixed correctly. This is what I have done in my gen 1 fix for haze and disable. By implementing this policy, we can build a better culture of writing unit tests so that bugs are found before reaching production, and doing so will also get people more familiar with the Showdown codebase.
Let me know your thoughts on this policy suggestion.
The text was updated successfully, but these errors were encountered:
gigalh128
changed the title
Policy Suggestion: Mechanics Fixing PRs Must Have Tests
Policy Suggestion: Require Mechanics Fixing PRs to Have Unit Tests
Aug 16, 2022
I support this. Unit tests are generally helpful and mechanics are relatively easy to test. I do think that for new contributors, we should make it clear that maintainers will help them to write unit tests (testing frameworks can be hard to wrap your head around as a new contributor).
I would prefer this as an encouraged suggestion, not mandatory. Certain small mechanics fixes don't warrant tests, and I wouldn't want someone to say, open a PR with "Prevent Dynamax Cannon from being called by Instruct" with a test where it's not really needed. I am in agreement with Annika that seasoned maintainers helping with tests, or straight up providing tests, is a great middle ground if it's a new developer.
The reason I've never required this is because it's kind of hostile to new contributors. "Thanks for helping us for free, now here's some more work for you."
I've also been on the other end of this ( mochajs/mocha#2746 ), where I submit a bugfix, and they demand specific kinds of tests that aren't possible, and the bug sits unfixed because that's the rules.
I agree with Wob; encouraging more unit tests is good, but I think it would be bad to make it a requirement.
Showdown has issues with buggy move mechanics being found in production. Ideally, we would have tests for moves to catch these before this happens. I know we can't feasibly write comprehensive tests to prevent all of these now, as there are just too many moves. However, we can mitigate this, and one simple idea is to require anyone who makes a PR fixing a certain mechanic to also make unit tests that demonstrate the mechanic has been fixed correctly. This is what I have done in my gen 1 fix for haze and disable. By implementing this policy, we can build a better culture of writing unit tests so that bugs are found before reaching production, and doing so will also get people more familiar with the Showdown codebase.
Let me know your thoughts on this policy suggestion.
The text was updated successfully, but these errors were encountered: