godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.16k stars 97 forks source link

Establishing naming conventions for test macros #1388

Open Xrayez opened 4 years ago

Xrayez commented 4 years ago

Describe the project you are working on: Goost - Godot Engine Extension.

Describe the problem or limitation you are having in your project: Inconsistency between error macros and test macros names, in terms of usage.

Describe the feature / enhancement and how it helps to overcome the problem or limitation: Provide aliases for doctest macros to follow Godot naming conventions.

If you look at core/error_macros.h, you'll see a set of macros which are currently used by the engine for handling errors. All are prefixed with ERR_*, and some have corresponding *_MSG macros for conveying error rationale.

doctest provides a default set of simple macros such as CHECK and REQUIRE (which are also aliases for DOCTEST_CHECK and DOCTEST_REQUIRE internally, by the way). Macros with rationale are quite verbose too, for instance: CHECK_MESSAGE.

Currently we have tests/test_macros.h which could also have similar macros as in core/error_macros.h defined specifically for testing. They would be prefixed with TEST_* and also have corresponding *_MSG macros.

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams: Here's a rename mapping I propose, feel free to suggest anything else.

Assertions

Sorted by severity (prefer CHECK macros as they do not fail the test immediately).

doctest Godot Description
DOCTEST_REQUIRE TEST_REQUIRE Test condition, fail the test if does not hold true.
DOCTEST_REQUIRE_MESSAGE TEST_REQUIRE_MSG Same as above, prints a rationale.
DOCTEST_REQUIRE_FALSE TEST_FAIL_COND Fail the test if condition holds true.
DOCTEST_REQUIRE_FALSE_MESSAGE TEST_FAIL_COND_MSG Same as above, prints a rationale.
DOCTEST_CHECK TEST_CHECK Simple assertion.
DOCTEST_CHECK_MESSAGE TEST_CHECK_MSG Same as above, prints a rationale.
DOCTEST_CHECK_FALSE TEST_COND Test if condition holds true, but does not fail the test.
DOCTEST_CHECK_FALSE_MESSAGE TEST_COND_MSG Same as above, prints a rationale.
DOCTEST_WARN TEST_WARN Test condition, does not fail the test, but logs a warning if something does not hold true.
DOCTEST_WARN_MESSAGE TEST_WARN_MSG Same as above, prints a rationale.
DOCTEST_WARN_FALSE TEST_COND_WARN Test condition, does not fail the test, but logs a warning if something holds true.
DOCTEST_WARN_FALSE_MESSAGE TEST_COND_WARN_MSG Same as above, prints a rationale.

Note that TEST_COND and TEST_FAIL_COND are already used locally in ClassDB tests, for instance.

Logging

doctest Godot Description
DOCTEST_MESSAGE TEST_MSG Print a message.
DOCTEST_FAIL_CHECK TEST_FAIL_CONTINUE Mark the test as failing, but continue the execution. Can be wrapped in conditionals for complex checks.
DOCTEST_FAIL TEST_FAIL Fail the test immediately. Can be wrapped in conditionals for complex checks.

I'd like to refactor all existing macros usages once we reach a consensus.

If this enhancement will not be used often, can it be worked around with a few lines of script?: C++ developers can still choose to use their own macros however they see it, or continue using doctest macros.

Is there a reason why this should be core and not an add-on in the asset library?: Have to modify tests/test_macros.h.

Calinou commented 3 years ago

I think the current names we use for Doctest macros are fine (CHECK_MESSAGE(), etc). We don't need to be very consistent as the unit tests' code is separated from the engine code.