search

Crypto-toolbox / bitex

Crypto-currency Exchange API Framework
MIT License
484 stars 136 forks source link

Fix travis / tests or temporarily disable tests #186

Open firepol opened 6 years ago

firepol commented 6 years ago

The dev branch doesn't look to pass any tests... I mean when we submit a PR it's kinda sad to see "All checks have failed" and it's not even our code's fault ;)

Would be nice to fix this stuff, so when somebody is submitting a PR, it's all green if it's all good, and it's red when a specific test was broken, after all, that's the purpose of unit testing I guess.

If tests always fail, nobody will check them and they are useless. Then they should be temporarily disabled until they are properly fixed.

deepbrook commented 6 years ago

Yepp, you're right - they need fixing. I disagree on them being useless, however. Anyone submitting their PRs should have run tests before submitting, making sure their tests pass, and no other tests break.

firepol commented 6 years ago

Don't take me wrong, I don't think that disabling ALL tests is the way to go. But say there are these 2-3 annoying tests that always fail, what's the point to have them fail all the time? Disable them and create an issue (so t won't be forgotten) in github to fix the problem and re-enable them later. So at least all the other ones, the ones that are green, will prevail and the build will be green again. So if a new PR will mess up with one of those tests, the build will be red because of a mistake in the PR, not because these 2-3 tests fail all the time...

Thx for considering this... it makes submitting a PR more fun then seeing "All checks have failed" ... "The Travis CI build failed"