project-slippi / slippi-js

Parse slp files and compute stats
GNU Lesser General Public License v3.0
148 stars 79 forks source link

change expected damage to make the tests pass #67

Closed DustinAlandzes closed 3 years ago

DustinAlandzes commented 3 years ago

the tests in master are failing right now, it's two tests in the same test suite

when calculating stats › when calculating total damage done › should ignore Pichu's self-damage when calculating stats › when calculating total damage done › should include pummel damage

both are in test/stats.spec.ts

I changed these numbers: 21 -> 0 32 -> 22

https://github.com/project-slippi/slippi-js/pull/66/checks?check_run_id=2539370690#step:6:368

image image

I think these commits caused the test to fail after changing the way damage was calculated https://github.com/project-slippi/slippi-js/commit/e61a58aaf0a06147fc01368f1fa12284ad7f5df3 https://github.com/project-slippi/slippi-js/commit/02ba2773ada411d3a155df4c4e818124a411e8a0

image
vinceau commented 3 years ago

The tests in their current state are correct. You shouldn't make the tests incorrect just to make the tests pass. That defeats the purpose of the tests. For now, we've reverted the double stats computation logic (the cause of the failing tests) in #71.