Skip to content

Adds Queen Attack test cases from x-common#473

Merged
Insti merged 1 commit intoexercism:masterfrom
samjonester:queen-attack
Oct 31, 2016
Merged

Adds Queen Attack test cases from x-common#473
Insti merged 1 commit intoexercism:masterfrom
samjonester:queen-attack

Conversation

@samjonester
Copy link
Copy Markdown
Contributor

No description provided.

@Insti
Copy link
Copy Markdown
Contributor

Insti commented Oct 26, 2016

Great, thanks.
Did you manually add the missing test cases?

@samjonester
Copy link
Copy Markdown
Contributor Author

I did. Would you like me to create a generator instead? This exercise has tests that aren't a part of x-common, related to presentation of the board, which would complicate the generator.

@Insti
Copy link
Copy Markdown
Contributor

Insti commented Oct 26, 2016

Updating the test file is a good start but if you would like to create a generator you are very welcome to, we need to do it at some point, it may as well be now.

The generator should only use the tests that are part of the x-common data. Any tests that are not part of x-common can be ignored.
If you think they are important and need to be included you can make a PR to add them to the canonical-data.json in x-common.

@samjonester
Copy link
Copy Markdown
Contributor Author

Cool! I'll make the generator with just the test cases from x-common. I think the extra tests confuse the purpose of the exercise anyways.

@Insti
Copy link
Copy Markdown
Contributor

Insti commented Oct 26, 2016

The documentation on how to create generators is a little out of date, so have a look at the recent Transpose test case generator #466 for an example of what to do.

The generator should have methods called: skipped, test_name, workload

@Insti Insti merged commit e4c5d1f into exercism:master Oct 31, 2016
@Insti
Copy link
Copy Markdown
Contributor

Insti commented Oct 31, 2016

Thanks @samjonester ❤️, I've merged in these changes so the test file is up to date.
You can create a new PR when you have made the generator.

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.

2 participants