Improve error messages in "robot-simulator"
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 45/100
- Issue type
- Feature
- Clarity
- Clearly specified
- Activity status
- Stale
- Tech stack
- cpp
- Domain
- testing-qa
Research direction
Start with robot_simulator_test.cpp and the existing Catch2 test setup. Review Catch2's StringMaker and CATCH_REGISTER_ENUM documentation, then add the proposed diagnostic support and run the robot-simulator tests; done means failed comparisons show pair values and Bearing names instead of placeholders or integers.
Written by the indexing model from the issue text.
Description
The tests in the exercise "robot-simulator" compare the result of the member functions get_position() and get_bearing() with the operator== like this:
TEST_CASE("A_robots_is_created_with_a_position_and_a_direction")
{
const Robot r;
const std::pair<int, int> expected_robot_position{0, 0};
REQUIRE(expected_robot_position == r.get_position());
REQUIRE(Bearing::NORTH == r.get_bearing());
}
But Catch2 does not know how to print a std::pair, and it prints an enum or enum class like an integer:
-------------------------------------------------------------------------------
A_robots_is_created_with_a_position_and_a_direction
-------------------------------------------------------------------------------
/home/user/exercism/cpp/robot-simulator/robot_simulator_test.cpp:13
...............................................................................
/home/user/exercism/cpp/robot-simulator/robot_simulator_test.cpp:18: FAILED:
REQUIRE( expected_robot_position == r.get_position() )
with expansion:
{?} == {?}
-------------------------------------------------------------------------------
A_robots_is_created_with_a_position_and_a_direction
-------------------------------------------------------------------------------
/home/user/exercism/cpp/robot-simulator/robot_simulator_test.cpp:13
...............................................................................
/home/user/exercism/cpp/robot-simulator/robot_simulator_test.cpp:19: FAILED:
REQUIRE( Bearing::NORTH == r.get_bearing() )
with expansion:
0 == 2
That's not really helpful.
Catch2 has the a StringMaker for printing custom classes (see the documentation), and it has the macro CATCH_REGISTER_ENUM() for better error messages when working with enums (see the documentation).
By adding a few lines somewhere at the beginning of robot_simulator_test.cpp
// for better error messages
namespace Catch
{
template <typename T1, typename T2>
struct StringMaker<std::pair<T1, T2>>
{
static std::string convert(const std::pair<T1, T2>& value)
{
std::string result = "std::pair{";
result += StringMaker<T1>::convert(value.first);
result += ", ";
result += StringMaker<T2>::convert(value.second);
result += '}';
return result;
}
};
}
CATCH_REGISTER_ENUM(robot_simulator::Bearing,
robot_simulator::Bearing::NORTH,
robot_simulator::Bearing::WEST,
robot_simulator::Bearing::SOUTH,
robot_simulator::Bearing::EAST)
we would get better error messages:
-------------------------------------------------------------------------------
A_robots_is_created_with_a_position_and_a_direction
-------------------------------------------------------------------------------
/home/user/exercism/cpp/robot-simulator/robot_simulator_test.cpp:36
...............................................................................
/home/user/exercism/cpp/robot-simulator/robot_simulator_test.cpp:41: FAILED:
REQUIRE( expected_robot_position == r.get_position() )
with expansion:
std::pair{0, 0} == std::pair{0, 1}
-------------------------------------------------------------------------------
A_robots_is_created_with_a_position_and_a_direction
-------------------------------------------------------------------------------
/home/user/exercism/cpp/robot-simulator/robot_simulator_test.cpp:36
...............................................................................
/home/user/exercism/cpp/robot-simulator/robot_simulator_test.cpp:42: FAILED:
REQUIRE( Bearing::NORTH == r.get_bearing() )
with expansion:
NORTH == SOUTH
AFAIK there are only two possible problems:
-
robot_simulator_test.cppbecomes more complex. But IMHO those 22 lines can be ignored. -
That would effectively enforce the use of
enumorenum classforBearing. IMHO that's not a problem for us because we want idiomatic solutions, and that'senumor betterenum class.
IMHO the benefits outweigh these problems.
What do you folks think?
- Dominant language
- C++
- Stars
- 291
- Forks
- 244
- Avg merge
- 17m
- Merged PRs (30d)
- 1
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from exercism/cpp
-
Difficulty 4/5 3-5 days Newbie friendliness 35/100
-
Difficulty 4/5 3-5 days Newbie friendliness 48/100
-
Difficulty 4/5 3-5 days Newbie friendliness 45/100
-
Difficulty 4/5 3-5 days Newbie friendliness 35/100
-
Difficulty 4/5 3-5 days Newbie friendliness 35/100
Similar issues
-
AuTest Bug Tests
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
apache/trafficserver#13714 ·
-
bug build
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
facebookincubator/velox#19143 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
tenstorrent/tt-metal#57393 · 1 comment ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
objectionary/eo-graphs#74 ·