Skip to content
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 some missing tests for the chess package #189

Closed

Conversation

adamkasztenny
Copy link
Contributor

Some tests were missing for some files in the chess package. These were low-hanging fruit, but there are still other files in this package and others that are untested.

"Mode" should {
"either be Casual or Rated" in {
Casual.casual must_== true
Casual.rated must_== false
Copy link
Collaborator

@ornicar ornicar Feb 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry but I don't understand the point of these tests, and other similar tests. How does that not just repeat the equally simple code it's supposed to test:

  def casual = this == Mode.Casual
  def rated  = this == Mode.Rated

I think tests for the sake of testing is not a good thing.

@ornicar
Copy link
Collaborator

ornicar commented Feb 16, 2020

I'm sorry that you put all that work in. I can't merge that. Most of these tests are just repeating the code, without possibly catching any bug in it. I don't see any value in them.
If I am missing something, please enlighten me.

@ornicar ornicar closed this Feb 16, 2020
@adamkasztenny adamkasztenny deleted the increase_coverage branch February 16, 2020 02:32
@adamkasztenny
Copy link
Contributor Author

adamkasztenny commented Feb 16, 2020

No problem, you're the boss. The reason I wrote these is that there are no tests covering these classes. There still is logic in the classes (if statements, filters, etc) so that's why I wrote them. Also, it didn't take me very long so don't worry about it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants