Skip to content

Add standard tests for Queen Attack#211

Merged
kytrinyx merged 1 commit intoexercism:masterfrom
IanWhitney:define_queen_attack
Mar 27, 2016
Merged

Add standard tests for Queen Attack#211
kytrinyx merged 1 commit intoexercism:masterfrom
IanWhitney:define_queen_attack

Conversation

@IanWhitney
Copy link
Copy Markdown
Contributor

https://github.com/exercism/todo/issues/133

While implementing Queen Attack for Rust
(exercism/rust#89), I surveyed all current
implementations of this exercise and documented which tests they used.
That full list of tests is here:
https://gist.github.com/IanWhitney/dd34f8a16c730f3cd77c

In order of prevalence, the five most common test types cover:

  1. Can 2 queens attack each other?
  2. Can 2 pieces occupy the same
    space
    ?
  3. Comparing string representations of the
    chessboard
  4. Does a chessboard start with the queens in their default
    positions?
  5. Errors are raised if an invalid position is
    used

Then there are some uncommon edge-case tests. Can there be more than one
chessboard? Can the board be empty? Algebraic chess notation? Etc.

Of the top five, I found that only numbers 1 and 5 really cover the
exercise as stated -- if I place two queens on the board, can they
attack each other?

So, those are the tests that I've documented in this json file.

Implementing the other tests requires the introduction of a Chessboard
object, and also a way to identify pieces (which team they belong to,
how they are represented as strings, etc.) That can be interesting to
do, but I find it beyond the scope of this exercise as written.

At best, I think these tests should be suggested as Extra Credit.

Test order also varied between implementations.

I have found that the faster I can get the first test to pass, the
better. So my first test requires nothing more than a can_attack
function that returns false. The tests then build on that, adding
complexity. Then, once can_attack is fully implemented, I add the
curveball test of invalid chess positions, which will get students
thinking about bad inputs and error handling.

@IanWhitney IanWhitney force-pushed the define_queen_attack branch from e3a8a7e to 5a9aec9 Compare March 27, 2016 16:22
https://github.com/exercism/todo/issues/133

While implementing Queen Attack for Rust
(exercism/rust#89), I surveyed all current
implementations of this exercise and documented which tests they used.
That full list of tests is here:
https://gist.github.com/IanWhitney/dd34f8a16c730f3cd77c

In order of prevalence, the five most common test types cover:

1. Can 2 queens attack each other?
2. [Can 2 pieces occupy the same
space](https://github.com/exercism/xelixir/blob/0965c4e107de68979a493b82f9f0859773fcb83b/exercises/queen-attack/queen_attack_test.exs#L26)?
3. [Comparing string representations of the
chessboard](https://github.com/exercism/xelixir/blob/0965c4e107de68979a493b82f9f0859773fcb83b/exercises/queen-attack/queen_attack_test.exs#L33)
4. [Does a chessboard start with the queens in their default
positions?](https://github.com/exercism/xelixir/blob/0965c4e107de68979a493b82f9f0859773fcb83b/exercises/queen-attack/queen_attack_test.exs#L12)
5. [Errors are raised if an invalid position is
used](https://github.com/exercism/xpython/blob/b9dfe291845a31301029d947b3cf3a2ffc85b0b6/exercises/queen-attack/queen_attack_test.py#L55)

Then there are some uncommon edge-case tests. Can there be more than one
chessboard? Can the board be empty? Algebraic chess notation? Etc.

Of the top five, I found that only numbers 1 and 5 really cover the
exercise as stated -- if I place two queens on the board, can they
attack each other?

So, those are the tests that I've documented in this json file.

Implementing the other tests requires the introduction of a Chessboard
object, and also a way to identify pieces (which team they belong to,
how they are represented as strings, etc.)  That can be interesting to
do, but I find it beyond the scope of this exercise as written.

At best, I think these tests should be suggested as Extra Credit.

Test order also varied between implementations.

I have found that the faster I can get the first test to pass, the
better. So my first test requires nothing more than a `can_attack`
function that returns false. The tests then build on that, adding
complexity. Then, once `can_attack` is fully implemented, I add the
curveball test of invalid chess positions, which will get students
thinking about bad inputs and error handling.
@IanWhitney IanWhitney force-pushed the define_queen_attack branch from 5a9aec9 to ed35c0f Compare March 27, 2016 16:23
@kytrinyx
Copy link
Copy Markdown
Member

This is great stuff! Thanks so much for conducting the cross-track survey.

I think that you've chosen a very good set of canonical tests, and that these reflect the problem as described in the README. The algebraic notation and graphical representations are not particularly interesting in general (though I'm sure they could be interesting in some languages or to some people).

@kytrinyx kytrinyx merged commit 78004f0 into exercism:master Mar 27, 2016
@kytrinyx
Copy link
Copy Markdown
Member

Would you be interested in writing up an issue that could be posted to all the tracks about the canonical test inputs/outputs and observations made during the survey?

@IanWhitney
Copy link
Copy Markdown
Contributor Author

@kytrinyx Sure. How's this?

Hello,

As part of the following issues:

#89
https://github.com/exercism/todo/issues/133

We'd like to have a standard set of tests for Queen Attack.

A set of standard tests was merged into x-common with #211 which you can find them here: https://github.com/exercism/x-common/blob/master/queen-attack.json

If this track is missing tests that are part of the standard set, implementing them will ensure students a more consistent experience between tracks. And if this track implements tests beyond the standard, can they be removed or clearly marked as "Extra Credit"?

@IanWhitney
Copy link
Copy Markdown
Contributor Author

Github auto formatting made that hard to copy & paste into Blazon. Let me try again.

Hello, 

As discussed in the following issues:

https://github.com/exercism/x-common/issues/89
https://github.com/exercism/todo/issues/133

We'd like to have a standard set of tests for Queen Attack. 

A set of standard tests was merged into x-common with https://github.com/exercism/x-common/pull/211 which you can find them here: https://github.com/exercism/x-common/blob/master/queen-attack.json

If this track is missing tests that are part of the standard set, implementing them will ensure students a more consistent experience between tracks. And if this track implements tests beyond the standard, can they be removed or clearly marked as "Extra Credit"?

@kytrinyx
Copy link
Copy Markdown
Member

Nice! How about fleshing it out a bit with some of the things mentioned in those issues? (Also, if this gets copy/pasted into a file, then the first line becomes the subject-line of the issue).

I've copy/pasted some things from various issues and fleshed things out a bit as well. What do you think? Too prescriptive?

queen-attack: update tests to match canonical test data

I recently completed a survey of the existing implementations of the Queen Attack problem
(Clojure, CoffeeScript, C++, C#, ECMAScript, Elixir, F#, Go, Haskell, JavaScript, Perl 5,
Python, Ruby, and Scala) per issue https://github.com/exercism/todo/issues/133

Out of that, it looks like there were five basic types of test that cover the problem as described
in the README:

1. Can 2 queens attack each other?
1. Can 2 pieces occupy the same space?
1. Comparing string representations of the chessboard
1. Does a chessboard start with the queens in their default positions?
1. Errors are raised if an invalid position is used

There were also some edge-cases that covered things like multiple chess boards, empty chess
boards, algebraic notation, and a string representation of the board.

The common test cases are now available as a canonical set of inputs/outputs for the problem:
https://github.com/exercism/x-common/blob/master/queen-attack.json

Please take a moment to review the implementation in this track to ensure that the basic set
of tests are all covered.

In addition, it would be worth discussing whether or not tests that fall outside of the basic set
should be removed or moved into a separate section marked as _extra credit_ or similar.

Comment thread queen-attack.json
"black_queen": {
"position": "(2,5)"
},
"expected": false
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should that be true? since their second coordinate are both 5

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The description also suggests that this is expected to be true.

@IanWhitney
Copy link
Copy Markdown
Contributor Author

@kytrinyx I think that looks good. The one change I'd make is that it mentions string representations both as one of the five test types, and also as an edge case.

And I'm not sure if tracks tend to follow the order of the tests as they appear in the JSON. As I mentioned in this PR, I tried to order them in a particular way. But we almost-immediately found that my order does not work well for Rust. So we might want to mention that test order is variable?

@kytrinyx
Copy link
Copy Markdown
Member

I'm not sure if tracks tend to follow the order of the tests as they appear in the JSON.

Yeah for the generated ones, definitely. For hand-crafted ones I don't know; probably?

Do you want to re-order it in the JSON? Do you think that the order for Rust would make sense for other languages?

@IanWhitney
Copy link
Copy Markdown
Contributor Author

It's going to be so language dependent. Rust makes you deal with error cases very differently than Ruby, or PHP, etc.

@kytrinyx
Copy link
Copy Markdown
Member

Hm. Right. Should we split out the error cases in a separate section so that people can either start with errors or start with happy-path?

@IanWhitney
Copy link
Copy Markdown
Contributor Author

Maybe. Is there another exercise that we could use as a reference? I toured through the json files before submitting this PR, but didn't see a standard way of handling this.

@kytrinyx
Copy link
Copy Markdown
Member

There's no standard way of handling it, but we have multiple sections in clock.json: https://github.com/exercism/x-common/blob/master/clock.json

IanWhitney pushed a commit to IanWhitney/x-common that referenced this pull request Mar 30, 2016
@IanWhitney IanWhitney deleted the define_queen_attack branch March 30, 2016 01:50
emcoding pushed a commit that referenced this pull request Nov 19, 2018
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.

4 participants