Skip to content

Common Test Suite for Robot Simulator #282

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

Merged
merged 4 commits into from
Jun 30, 2016

Conversation

IanWhitney
Copy link
Contributor

Looking at the tracks that currently implement this problem the core tests seemed
to be

  • Turn and check bearing/direction
  • Place and check coordinate/position
  • Move and check bearing & coordinates

The F# track (along with some others) had what seemed to be the most thorough yet small test suite, so I've largely copied their approach here.

There were some variant tests that I haven't included, but that I mentioned in the descriptin.

  • Handling invalid directions
  • Handling invalid instructions
  • Defaults for position and direction

These are good things to have, but they aren't currently mentioned in the readme, so I left those tests out.

The Go implementation is a particular outlier because it was written for a totally different version of the Readme, so I didn't include any of its additional tests.

Looking at the tracks that currently implement this problem the core tests seemed
to be

- Turn and check bearing/direction
- Place and check coordinate/position
- Move and check bearing & coordinates

The [F# track](http://exercism.io/exercises/fsharp/robot-simulator)
(along with some others) had what seemed to be the most useful yet small
test suite, so I've largely copied their approach here.

There were some variant tests that I haven't included, but that I
mentioned in the descriptin.

- Handling invalid directions
- Handling invalid instructions
- Defaults for position and direction

These are good things to have, but they aren't currently mentioned in
the readme, so I left those tests out.

The [Go
implementation](http://exercism.io/exercises/go/robot-simulator/readme)
is a particular outlier because it was written for a [totally different
version of the
Readme](exercism@8aaf653),
so I didn't include any of its additional tests.
"direction": "north"
},
"expected": {
"position": "east"
Copy link
Member

Choose a reason for hiding this comment

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

direction

@petertseng
Copy link
Member

Tests look reasonable. What do you/everyone think about having the coordinates be e.g. [-1, -1] instead of "(-1, -1)" to avoid making tracks parse out the parentheses and numbers?

@IanWhitney
Copy link
Contributor Author

It doesn't matter to me. I picked parens because that's what I used in queen-attack.json.

But if square brackets are better for languages that auto-generate tests, then that works for me.

IanWhitney pushed a commit to IanWhitney/xrust that referenced this pull request Jun 27, 2016
Tests come from exercism/problem-specifications#282

As discussed in exercism#117 we are
providing a stub implementation so that ignored tests do not fail when a
student hasn't yet implemented the functionality they exercise.
@petertseng
Copy link
Member

Now you're making me wonder why I didn't ask the same in queen-attack.json. I do think [1, 2] would be better, wonder what others think (what tracks are generating their tests? I only know of Ruby and Go right now)

@IanWhitney
Copy link
Contributor Author

You can't catch everything, Peter.

@kotp or @kytrinyx, opinions on this? Knowledge of other languages that are auto-generating tests?

@kotp
Copy link
Member

kotp commented Jun 28, 2016

One communicates to me that the passed in argument needs to be an Array, the other suggests it could be an array or two arguments. Hopefully, the tests end up not forcing either, to keep the ways it can be solved as free as possible. The "(1, 2)", to me suggests arguments, where "[1, 2]" suggests using an array as the argument.

This is weighted against the title of the test and any 'message' given for when the test does not pass, as well as any comment for that section.

They are generated, so really, we can manipulate them as we need to. Clarity in the communication, even little things like parenthesis and brackets, counts though.

Hmm... the question though is in which place are you suggesting square brackets? In the expected? That communicates differently, than in the "position" spots.

      "position": "(0,0)",
            "direction": "east"
          },
 -        "expected": "(1,0)"

FYI @exercism/ruby is the 'ping' trigger that was setup for the Ruby track.

@kytrinyx
Copy link
Member

I don't have a preference. I would think that any track that implements this will think through what they want the API to be whether we choose square brackets or parentheses.

@IanWhitney
Copy link
Contributor Author

IanWhitney commented Jun 28, 2016

@kotp The Rust implementation will be treating them as parameters. When I did Queen Attack I passed the coordinates in as a tuple and people didn't like it. So I'm sticking with passing them in separately.

Sounds like the parens/bracket thing isn't a big concern. I'll let this sit for a few days before merging. Or, if any other maintainers want to merge, go for it. I'm not the boss of you!

@IanWhitney IanWhitney merged commit 2d6e205 into exercism:master Jun 30, 2016
@IanWhitney IanWhitney deleted the robot_simulator_test_suite branch June 30, 2016 02:02
emcoding pushed a commit that referenced this pull request Nov 19, 2018
This has the non-comparitive tests implemented from the changes
implemented in the canonical changes introduced by @kytrinyx
references #282
references #166
emcoding pushed a commit that referenced this pull request Nov 19, 2018
Now generates tests from data in x-common.

The template file is skinny, the generator a little fatter.

fixes #282
references #166
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.

5 participants