diff --git a/engine/compatibility_breakages.rst b/engine/compatibility_breakages.rst index 5747562..344f799 100644 --- a/engine/compatibility_breakages.rst +++ b/engine/compatibility_breakages.rst @@ -5,140 +5,4 @@ Handling compatibility breakages .. TODO: Elaborate on types of compatibility and procedure. -So you've added a new parameter to a method, changed the return type, -changed the type of a parameter, or changed its default value, -and now the automated testing is complaining about compatibility breakages? - -Breaking compatibility should be avoided, but when necessary there are systems in place -to handle this in a way that makes the transition as smooth as possible. - -A practical example -------------------- - -.. TODO: Add example that showcases more details like original default arguments etc. - -These changes are taken from `pull request #88047 `_, which added -new pathing options to ``AStarGrid2D`` and other AStar classes. -Among other changes, these methods were modified in ``core/math/a_star_grid_2d.h``: - -.. code-block:: cpp - - Vector get_point_path(const Vector2i &p_from, const Vector2i &p_to); - TypedArray get_id_path(const Vector2i &p_from, const Vector2i &p_to); - -To: - -.. code-block:: cpp - - Vector get_point_path(const Vector2i &p_from, const Vector2i &p_to, bool p_allow_partial_path = false); - TypedArray get_id_path(const Vector2i &p_from, const Vector2i &p_to, bool p_allow_partial_path = false); - -This meant adding new compatibility method bindings to the file, which should be in the ``protected`` section of -the code, usually placed next to ``_bind_methods()``: - -.. code-block:: cpp - - #ifndef DISABLE_DEPRECATED - TypedArray _get_id_path_bind_compat_88047(const Vector2i &p_from, const Vector2i &p_to); - Vector _get_point_path_bind_compat_88047(const Vector2i &p_from, const Vector2i &p_to); - static void _bind_compatibility_methods(); - #endif - -They should start with an ``_`` to indicate that they are internal, and end with ``_bind_compat_`` followed by the PR number -that introduced the change (``88047`` in this example). These compatibility methods need to be implemented in a dedicated file, -like ``core/math/a_star_grid_2d.compat.inc`` in this case: - -.. code-block:: cpp - :caption: core/math/a_star_grid_2d.compat.inc - - /**************************************************************************/ - /* a_star_grid_2d.compat.inc */ - /**************************************************************************/ - /* This file is part of: */ - /* GODOT ENGINE */ - /* https://godotengine.org */ - /**************************************************************************/ - /* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */ - /* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur. */ - /* */ - /* Permission is hereby granted, free of charge, to any person obtaining */ - /* a copy of this software and associated documentation files (the */ - /* "Software"), to deal in the Software without restriction, including */ - /* without limitation the rights to use, copy, modify, merge, publish, */ - /* distribute, sublicense, and/or sell copies of the Software, and to */ - /* permit persons to whom the Software is furnished to do so, subject to */ - /* the following conditions: */ - /* */ - /* The above copyright notice and this permission notice shall be */ - /* included in all copies or substantial portions of the Software. */ - /* */ - /* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */ - /* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */ - /* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */ - /* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */ - /* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */ - /* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */ - /* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ - /**************************************************************************/ - - #ifndef DISABLE_DEPRECATED - - #include "core/variant/typed_array.h" - - TypedArray AStarGrid2D::_get_id_path_bind_compat_88047(const Vector2i &p_from_id, const Vector2i &p_to_id) { - return get_id_path(p_from_id, p_to_id, false); - } - - Vector AStarGrid2D::_get_point_path_bind_compat_88047(const Vector2i &p_from_id, const Vector2i &p_to_id) { - return get_point_path(p_from_id, p_to_id, false); - } - - void AStarGrid2D::_bind_compatibility_methods() { - ClassDB::bind_compatibility_method(D_METHOD("get_id_path", "from_id", "to_id"), &AStarGrid2D::_get_id_path_bind_compat_88047); - ClassDB::bind_compatibility_method(D_METHOD("get_point_path", "from_id", "to_id"), &AStarGrid2D::_get_point_path_bind_compat_88047); - } - - #endif // DISABLE_DEPRECATED - -Unless the change in compatibility is complex, the compatibility method should call the modified method directly, -instead of duplicating that method. Make sure to match the default arguments for that method (in the example above this would be ``false``). - -This file should always be placed next to the original file, and have ``.compat.inc`` at the end instead of ``.cpp`` or ``.h``. -Next, this should be included in the ``.cpp`` file we're adding compatibility methods to, so ``core/math/a_star_grid_2d.cpp``: - -.. code-block:: cpp - :caption: core/math/a_star_grid_2d.cpp - - #include "a_star_grid_2d.h" - #include "a_star_grid_2d.compat.inc" - - #include "core/variant/typed_array.h" - -And finally, the changes reported by the API validation step should be added to the relevant validation file. Because this was -done during the development of 4.3, this would be ``misc/extension_api_validation/4.2-stable.expected`` (including changes not shown in -this example): - -.. code-block:: text - :caption: misc/extension_api_validation/4.2-stable.expected - - GH-88047 - -------- - Validate extension JSON: Error: Field 'classes/AStar2D/methods/get_id_path/arguments': size changed value in new API, from 2 to 3. - Validate extension JSON: Error: Field 'classes/AStar2D/methods/get_point_path/arguments': size changed value in new API, from 2 to 3. - Validate extension JSON: Error: Field 'classes/AStar3D/methods/get_id_path/arguments': size changed value in new API, from 2 to 3. - Validate extension JSON: Error: Field 'classes/AStar3D/methods/get_point_path/arguments': size changed value in new API, from 2 to 3. - Validate extension JSON: Error: Field 'classes/AStarGrid2D/methods/get_id_path/arguments': size changed value in new API, from 2 to 3. - Validate extension JSON: Error: Field 'classes/AStarGrid2D/methods/get_point_path/arguments': size changed value in new API, from 2 to 3. - - Added optional "allow_partial_path" argument to get_id_path and get_point_path methods in AStar classes. - Compatibility methods registered. - -The instructions for how to add to that file are at the top of the file itself. - -If you get a "Hash changed" error for a method, it means that the compatibility binding is missing or incorrect. -Such lines shouldn't be added to the ``.expected`` file, but fixed by binding the proper compatibility method. - -And that's it! You might run into a bit more complicated cases, like rearranging arguments, -changing return types, etc., but this covers the basic on how to use this system. - -For more information, see `pull request #76446 `_. +See also the `current documentation for compatibility breakages `_. diff --git a/engine/unit_tests.rst b/engine/unit_tests.rst index 87abf94..0104554 100644 --- a/engine/unit_tests.rst +++ b/engine/unit_tests.rst @@ -3,453 +3,4 @@ Unit testing ============ -Godot Engine allows to write unit tests directly in C++. The engine integrates -the `doctest `_ unit testing framework which -gives ability to write test suites and test cases next to production code, but -since the tests in Godot go through a different ``main`` entry point, the tests -reside in a dedicated ``tests/`` directory instead, which is located at the root -of the engine source code. - -Platform and target support ---------------------------- - -C++ unit tests can be run on Linux, macOS, and Windows operating systems. - -Tests can only be run with editor ``tools`` enabled, which means that export -templates cannot be tested currently. - -Running tests -------------- - -Before tests can be actually run, the engine must be compiled with the ``tests`` -build option enabled (and any other build option you typically use), as the -tests are not compiled as part of the engine by default: - -.. code-block:: shell - - scons tests=yes - -Once the build is done, run the tests with a ``--test`` command-line option: - -.. code-block:: shell - - ./bin/ --test - -The test run can be configured with the various doctest-specific command-line -options. To retrieve the full list of supported options, run the ``--test`` -command with the ``--help`` option: - -.. code-block:: shell - - ./bin/ --test --help - -Any other options and arguments after the ``--test`` command are treated as -arguments for doctest. - -.. note:: - - Tests are compiled automatically if you use the ``dev_mode=yes`` SCons option. - ``dev_mode=yes`` is recommended if you plan on contributing to the engine - development as it will automatically treat compilation warnings as errors. - The continuous integration system will fail if any compilation warnings are - detected, so you should strive to fix all warnings before opening a pull - request. - -Filtering tests -~~~~~~~~~~~~~~~ - -By default, all tests are run if you don't supply any extra arguments after the -``--test`` command. But if you're writing new tests or would like to see the -successful assertions output coming from those tests for debugging purposes, you -can run the tests of interest with the various filtering options provided by -doctest. - -The wildcard syntax ``*`` is supported for matching any number of characters in -test suites, test cases, and source file names: - -+--------------------+---------------+------------------------+ -| **Filter options** | **Shorthand** | **Examples** | -+--------------------+---------------+------------------------+ -| ``--test-suite`` | ``-ts`` | ``-ts="*[GDScript]*"`` | -+--------------------+---------------+------------------------+ -| ``--test-case`` | ``-tc`` | ``-tc="*[String]*"`` | -+--------------------+---------------+------------------------+ -| ``--source-file`` | ``-sf`` | ``-sf="*test_color*"`` | -+--------------------+---------------+------------------------+ - -For instance, to run only the ``String`` unit tests, run: - -.. code-block:: shell - - ./bin/ --test --test-case="*[String]*" - -Successful assertions output can be enabled with the ``--success`` (``-s``) -option, and can be combined with any combination of filtering options above, -for instance: - -.. code-block:: shell - - ./bin/ --test --source-file="*test_color*" --success - -Specific tests can be skipped with corresponding ``-exclude`` options. As of -now, some tests include random stress tests which take a while to execute. In -order to skip those kind of tests, run the following command: - -.. code-block:: shell - - ./bin/ --test --test-case-exclude="*[Stress]*" - -Writing tests -------------- - -Test suites represent C++ header files which must be included as part of the -main test entry point in ``tests/test_main.cpp``. Most test suites are located -directly under ``tests/`` directory. - -All header files are prefixed with ``test_``, and this is a naming convention -which the Godot build system relies on to detect tests throughout the engine. - -Here's a minimal working test suite with a single test case written: - -.. code-block:: cpp - - #pragma once - - #include "tests/test_macros.h" - - namespace TestString { - - TEST_CASE("[String] Hello World!") { - String hello = "Hello World!"; - CHECK(hello == "Hello World!"); - } - - } // namespace TestString - -.. note:: - You can quickly generate new tests using the ``create_test.py`` script found in the ``tests/`` directory. - This script automatically creates a new test file with the required boilerplate code in the appropriate location. - It's also able to automatically include the new header in ``tests/test_main.cpp`` using invasive mode (``-i`` flag). - To view usage instructions, run the script with the ``-h`` flag. - -The ``tests/test_macros.h`` header encapsulates everything which is needed for -writing C++ unit tests in Godot. It includes doctest assertion and logging -macros such as ``CHECK`` as seen above, and of course the definitions for -writing test cases themselves. - -.. seealso:: - - `tests/test_macros.h `_ - source code for currently implemented macros and aliases for them. - -Test cases are created using ``TEST_CASE`` function-like macro. Each test case -must have a brief description written in parentheses, optionally including -custom tags which allow to filter the tests at runtime, such as ``[String]``, -``[Stress]`` etc. - -Test cases are written in a dedicated namespace. This is not required, but -allows to prevent naming collisions for when other static helper functions are -written to accommodate the repeating testing procedures such as populating -common test data for each test, or writing parameterized tests. - -Godot supports writing tests per C++ module. For instructions on how to write -module tests, refer to :ref:`doc_custom_module_unit_tests`. - -Subcases -~~~~~~~~ - -In situations where you have a common setup for several test cases with only slight variations, subcases can be very helpful. Here's an example: - -.. code-block:: cpp - - TEST_CASE("[SceneTree][Node] Testing node operations with a very simple scene tree") { - // ... common setup (e.g. creating a scene tree with a few nodes) - SUBCASE("Move node to specific index") { - // ... setup and checks for moving a node - } - SUBCASE("Remove node at specific index") { - // ... setup and checks for removing a node - } - } - -Each ``SUBCASE`` causes the ``TEST_CASE`` to be executed from the beginning. -Subcases can be nested to an arbitrary depth, but it is advised to limit nesting to no more than one level deep. - -Assertions -~~~~~~~~~~ - -A list of all commonly used assertions used throughout the Godot tests, sorted -by severity. - -+-------------------+----------------------------------------------------------------------------------------------------------------------------------+ -| **Assertion** | **Description** | -+-------------------+----------------------------------------------------------------------------------------------------------------------------------+ -| ``REQUIRE`` | Test if condition holds true. Fails the entire test immediately if the condition does not hold true. | -+-------------------+----------------------------------------------------------------------------------------------------------------------------------+ -| ``REQUIRE_FALSE`` | Test if condition does not hold true. Fails the entire test immediately if the condition holds true. | -+-------------------+----------------------------------------------------------------------------------------------------------------------------------+ -| ``CHECK`` | Test if condition holds true. Marks the test run as failing, but allow to run other assertions. | -+-------------------+----------------------------------------------------------------------------------------------------------------------------------+ -| ``CHECK_FALSE`` | Test if condition does not hold true. Marks the test run as failing, but allow to run other assertions. | -+-------------------+----------------------------------------------------------------------------------------------------------------------------------+ -| ``WARN`` | Test if condition holds true. Does not fail the test under any circumstance, but logs a warning if something does not hold true. | -+-------------------+----------------------------------------------------------------------------------------------------------------------------------+ -| ``WARN_FALSE`` | Test if condition does not hold true. Does not fail the test under any circumstance, but logs a warning if something holds true. | -+-------------------+----------------------------------------------------------------------------------------------------------------------------------+ - -All of the above assertions have corresponding ``*_MESSAGE`` macros, which allow -to print optional message with rationale of what should happen. - -Prefer to use ``CHECK`` for self-explanatory assertions and ``CHECK_MESSAGE`` -for more complex ones if you think that it deserves a better explanation. - -.. seealso:: - - `doctest: Assertion macros `_. - -Logging -~~~~~~~ - -The test output is handled by doctest itself, and does not rely on Godot -printing or logging functionality at all, so it's recommended to use dedicated -macros which allow to log test output in a format written by doctest. - -+----------------+-----------------------------------------------------------------------------------------------------------+ -| **Macro** | **Description** | -+----------------+-----------------------------------------------------------------------------------------------------------+ -| ``MESSAGE`` | Prints a message. | -+----------------+-----------------------------------------------------------------------------------------------------------+ -| ``FAIL_CHECK`` | Marks the test as failing, but continue the execution. Can be wrapped in conditionals for complex checks. | -+----------------+-----------------------------------------------------------------------------------------------------------+ -| ``FAIL`` | Fails the test immediately. Can be wrapped in conditionals for complex checks. | -+----------------+-----------------------------------------------------------------------------------------------------------+ - -Different reporters can be chosen at runtime. For instance, here's how the -output can be redirected to an XML file: - -.. code-block:: shell - - ./bin/ --test --source-file="*test_validate*" --success --reporters=xml --out=doctest.txt - -.. seealso:: - - `doctest: Logging macros `_. - -Testing failure paths -~~~~~~~~~~~~~~~~~~~~~ - -Sometimes, it's not always feasible to test for an *expected* result. With the -Godot development philosophy of that the engine should not crash and should -gracefully recover whenever a non-fatal error occurs, it's important to check -that those failure paths are indeed safe to execute without crashing the engine. - -*Unexpected* behavior can be tested in the same way as anything else. The only -problem this creates is that the error printing shall unnecessarily pollute the -test output with errors coming from the engine itself (even if the end result is -successful). - -To alleviate this problem, use ``ERR_PRINT_OFF`` and ``ERR_PRINT_ON`` macros -directly within test cases to temporarily disable the error output coming from -the engine, for instance: - -.. code-block:: cpp - - TEST_CASE("[Color] Constructor methods") { - ERR_PRINT_OFF; - Color html_invalid = Color::html("invalid"); - ERR_PRINT_ON; // Don't forget to re-enable! - - CHECK_MESSAGE(html_invalid.is_equal_approx(Color()), - "Invalid HTML notation should result in a Color with the default values."); - } - -Special tags in test case names -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -These tags can be added to the test case name to modify or extend the test environment: - -+--------------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------+ -| **Tag** | **Description** | -+--------------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------+ -| ``[SceneTree]`` | Required for test cases that rely on a scene tree with MessageQueue to be available. It also enables a mock rendering server and :ref:`ThemeDB`. | -+--------------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------+ -| ``[Editor]`` | Like ``[SceneTree]``, but with additional editor-related infrastructure available, such as :ref:`EditorSettings`. | -+--------------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------+ -| ``[Audio]`` | Initializes the :ref:`AudioServer` using a mock audio driver. | -+--------------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------+ -| ``[Navigation2D]`` | Creates the default 2D navigation server and makes it available for testing. | -+--------------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------+ -| ``[Navigation3D]`` | Creates the default 3D navigation server and makes it available for testing. | -+--------------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------+ - -You can use them together to combine multiple test environment extensions. - -Testing signals -~~~~~~~~~~~~~~~ - -The following macros can be use to test signals: - -.. list-table:: - :header-rows: 1 - :widths: auto - - * - Macro - - Description - * - ``SIGNAL_WATCH(object, "signal_name")`` - - Starts watching the specified signal on the given object. - * - ``SIGNAL_UNWATCH(object, "signal_name")`` - - Stops watching the specified signal on the given object. - * - ``SIGNAL_CHECK("signal_name", Vector>)`` - - Checks the arguments of all fired signals. The outer vector contains each fired signal, while the inner vector contains the list of arguments for that signal. The order of signals is significant. - * - ``SIGNAL_CHECK_FALSE("signal_name")`` - - Checks if the specified signal was not fired. - * - ``SIGNAL_DISCARD("signal_name")`` - - Discards all records of the specified signal. - -Below is an example demonstrating the use of these macros: - -.. code-block:: cpp - - //... - SUBCASE("[Timer] Timer process timeout signal must be emitted") { - SIGNAL_WATCH(test_timer, SNAME("timeout")); - test_timer->start(0.1); - - SceneTree::get_singleton()->process(0.2); - - Array signal_args; - signal_args.push_back(Array()); - - SIGNAL_CHECK(SNAME("timeout"), signal_args); - - SIGNAL_UNWATCH(test_timer, SNAME("timeout")); - } - //... - -Test tools ----------- - -Test tools are advanced methods which allow you to run arbitrary procedures to -facilitate the process of manual testing and debugging the engine internals. - -These tools can be run by supplying the name of a tool after the ``--test`` -command-line option. For instance, the GDScript module implements and registers -several tools to help the debugging of the tokenizer, parser, and compiler: - -.. code-block:: shell - - ./bin/ --test gdscript-tokenizer test.gd - ./bin/ --test gdscript-parser test.gd - ./bin/ --test gdscript-compiler test.gd - -If any such tool is detected, then the rest of the unit tests are skipped. - -Test tools can be registered anywhere throughout the engine as the registering -mechanism closely resembles of what doctest provides while registering test -cases using dynamic initialization technique, but usually these can be -registered at corresponding ``register_types.cpp`` sources (per module or core). - -Here's an example of how GDScript registers test tools in -``modules/gdscript/register_types.cpp``: - -.. code-block:: cpp - - #ifdef TESTS_ENABLED - void test_tokenizer() { - TestGDScript::test(TestGDScript::TestType::TEST_TOKENIZER); - } - - void test_parser() { - TestGDScript::test(TestGDScript::TestType::TEST_PARSER); - } - - void test_compiler() { - TestGDScript::test(TestGDScript::TestType::TEST_COMPILER); - } - - REGISTER_TEST_COMMAND("gdscript-tokenizer", &test_tokenizer); - REGISTER_TEST_COMMAND("gdscript-parser", &test_parser); - REGISTER_TEST_COMMAND("gdscript-compiler", &test_compiler); - #endif - -The custom command-line parsing can be performed by a test tool itself with the -help of OS :ref:`get_cmdline_args` method. - -Integration tests for GDScript ------------------------------- - -Godot uses doctest to prevent regressions in GDScript during development. There -are several types of test scripts which can be written: - -- tests for expected errors; -- tests for warnings; -- tests for features. - -Therefore, the process of writing integration tests for GDScript is the following: - -1. Pick a type of a test script you'd like to write, and create a new GDScript - file under the ``modules/gdscript/tests/scripts`` directory within - corresponding sub-directory. - -2. Write GDScript code. The test script must have a function called ``test()`` - which takes no arguments. Such function will be called by the test runner. - The test should not have any dependency unless it's part of the test too. - Global classes (using ``class_name``) are registered before the runner - starts, so those should work if needed. - - Here's an example test script: - - :: - - func test(): - if true # Missing colon here. - print("true") - -3. Change directory to the Godot source repository root. - - .. code-block:: shell - - cd godot - -4. Generate ``*.out`` files to update the expected results from the output: - - .. code-block:: shell - - bin/ --gdscript-generate-tests modules/gdscript/tests/scripts - -You may add the ``--print-filenames`` option to see filenames as their test -outputs are generated. If you are working on a new feature that is causing -hard crashes, you can use this option to quickly find which test file causes -the crash and debug from there. - -5. Run GDScript tests with: - - .. code-block:: shell - - ./bin/ --test --test-suite="*GDScript*" - -This also accepts the ``--print-filenames`` option (see above). - -If no errors are printed and everything goes well, you're done! - -.. warning:: - - Make sure the output does have the expected values before submitting a pull - request. If ``--gdscript-generate-tests`` produces ``*.out`` files which are - unrelated to newly added tests, you should revert those files back and - only commit ``*.out`` files for new tests. - -.. note:: - - The GDScript test runner is meant for testing the GDScript implementation, - not for testing user scripts nor testing the engine using scripts. We - recommend writing new tests for already resolved - `issues related to GDScript at GitHub `_, - or writing tests for currently working features. - -.. note:: - - If your test case requires that there is no ``test()`` - function present inside the script file, - you can disable the runtime section of the test by naming the script file so that it matches the pattern ``*.notest.gd``. - For example, "test_empty_file.notest.gd". +See also the `current documentation for unit tests `_. diff --git a/index.rst b/index.rst index a04dc4a..3ea52fb 100644 --- a/index.rst +++ b/index.rst @@ -17,7 +17,6 @@ TODO engine/cpp_usage_guidelines engine/code_style engine/compatibility_breakages - engine/unit_tests .. toctree:: :hidden: @@ -37,6 +36,14 @@ TODO proposals/about proposals/guidelines +.. toctree:: + :hidden: + :maxdepth: 1 + :caption: Testing and reporting issues + :name: sec-reporting-issues + + reporting_issues/first_steps + .. toctree:: :hidden: :maxdepth: 1 diff --git a/reporting_issues/first_steps.rst b/reporting_issues/first_steps.rst new file mode 100644 index 0000000..b314157 --- /dev/null +++ b/reporting_issues/first_steps.rst @@ -0,0 +1,36 @@ +Getting Started +=============== + +Another great way of contributing to the engine is to test development releases +or the development branch and to report issues. It is also helpful to report +issues discovered in stable releases, so that they can be fixed in +the development branch and in future maintenance releases. + +Testing development versions +---------------------------- + +To help with the testing, you have several possibilities: + +- Compile the engine from source yourself, following the instructions of the + :ref:`Compiling ` page for your platform. + +- Test official pre-release binaries when they are announced (usually on the + blog and other community platforms), such as alpha, beta and release candidate (RC) builds. + +- Test "trusted" unofficial builds of the development branch; just ask + community members for reliable providers. Whenever possible, it's best to + use official binaries or to compile yourself though, to be sure about the + provenance of your binaries. + +As mentioned previously, it is also helpful to keep your eyes peeled for +potential bugs that might still be present in the stable releases, especially +when using some niche features of the engine which might get less testing by +the developers. + +Filing an issue on GitHub +------------------------- + +Godot uses `GitHub's issue tracker `_ +for bug reports. When you start filing a bug report, you’ll be given a form to +fill out. Please try to follow it so that all issues are consistent and provide +the required information. diff --git a/triage/about.rst b/triage/about.rst index ab86bb8..cb86856 100644 --- a/triage/about.rst +++ b/triage/about.rst @@ -1,36 +1,287 @@ -Issue Triage -============ +.. _doc_bug_triage_guidelines: -Another great way of contributing to the engine is to test development releases -or the development branch and to report issues. It is also helpful to report -issues discovered in stable releases, so that they can be fixed in -the development branch and in future maintenance releases. +Bug triage guidelines +===================== -Testing development versions ----------------------------- +This page describes the typical workflow of the bug triage team aka +bugsquad when handling issues and pull requests on Godot's +`GitHub repository `__. +It is bound to evolve together with the bugsquad, so do not +hesitate to propose modifications to the following guidelines. -To help with the testing, you have several possibilities: +Issues management +----------------- -- Compile the engine from source yourself, following the instructions of the - :ref:`Compiling ` page for your platform. +For issue management, we use the following GitHub processes: -- Test official pre-release binaries when they are announced (usually on the - blog and other community platforms), such as alpha, beta and release candidate (RC) builds. +- Each issue and pull request (PR) is categorized with a set of *labels*, + sometimes called "tags". +- Each PR is assigned to a *milestone*. Some issues can also be assigned to a + *milestone* (see below). +- Issues can have an *assignee*, who is a contributor among Godot maintainers. +- Issues can be put in one or more *projects*. +- PRs can be *linked* to one or more issues which they "fix" or "close". -- Test "trusted" unofficial builds of the development branch; just ask - community members for reliable providers. Whenever possible, it's best to - use official binaries or to compile yourself though, to be sure about the - provenance of your binaries. +We don't yet extensively use or rely on some other GitHub processes: -As mentioned previously, it is also helpful to keep your eyes peeled for -potential bugs that might still be present in the stable releases, especially -when using some niche features of the engine which might get less testing by -the developers. +- Issue close reasons (completed, not planned, duplicate). While we use these, + it is not consistent, and older issues are all closed as "completed", so the + issue close reason should not be relied on. +- Issue *types* (Bug, Feature, Task). +- Issue *relationships*. -Filing an issue on GitHub -------------------------- +We only use the assignees feature for Godot maintainers who are members of the +Godot Engine GitHub organization, and even then not in all cases. For other +issues, we track who is working on an issue by comments on the issue and linked +pull requests. Most issues are available for any contributor to take on, after +discussing it with other contributors. If you would like to work on an issue, +first check that no one else is working on it, by looking for a linked pull +request, a comment "claiming" the issue, or an assignee. If no one else is +working on the issue, leave a comment on the issue to "claim" it and start +working on it. -Godot uses `GitHub's issue tracker `_ -for bug reports. When you start filing a bug report, you’ll be given a form to -fill out. Please try to follow it so that all issues are consistent and provide -the required information. +Labels +~~~~~~ + +The following `labels `__ are +currently defined in the Godot repository: + +Categories: +^^^^^^^^^^^ + +- *Archived*: used to filter issues closed with a resolution other than "fixed". + + - For issues, added to all issues that are not resolved by engine or + documentation changes. This includes duplicate issues, user error, or + reports in the wrong repository. + Since we don't rely on GitHub's issue close reasons (``completed``, ``not + planned``, and ``duplicate``), it is possible for an issue to be closed as + ``completed`` with the *Archived* label. + - For PRs, added to all closed PRs that are not merged. This includes superseded + or duplicate PRs, Git or GitHub mistakes, and valid PRs that end up not merged. + +- *Breaks compat*: describes something that can only be fixed by breaking + compatibility with existing projects. +- *Bug*: describes something that is not working properly. +- *Cherrypick*: describes something that can be backported to a stable branch + after being merged in the ``master`` branch. +- *Confirmed*: has been confirmed by at least one other contributor + than the bug reporter (typically for *Bug* reports). + The purpose of this label is to let developers know which issues are + still reproducible when they want to select what to work on. It is + therefore a good practice to add in a comment on what platform and + what version or commit of Godot the issue could be reproduced; if a + developer looks at the issue one year later, the *Confirmed* label + may not be relevant anymore. +- *Crash:* describes a bug that causes the engine to crash. + This label is only used for "hard" crashes, not freezes. +- *Discussion*: the issue is not consensual and needs further + discussion to define what exactly should be done to address the + topic. +- *Documentation*: related to the documentation. PRs with this label improve the + class reference. Issues with this label are either for wrong documentation, or + are user-reported "bugs" that are actually limitations to be further documented. + Often paired with *Discussion*. Issues related to the ReadTheDocs documentation + should be filed on the `godot-docs `_ repository. +- *Enhancement*: describes a proposed enhancement to an existing + functionality. +- *Feature proposal*: used for PRs adding new features which do not have a + corresponding proposal use this label. The label is removed when a feature + proposal is created and linked. The main Godot repository no longer accepts + feature requests as issues. Please use the `godot-proposals + `__ repository instead. +- *For PR meeting*: the issue needs to be discussed in a pull request meeting. + These meetings are public and are held on the `Godot Contributors Chat `_. +- *Good first issue*: the issue is *assumed* to be an easy one to fix, which makes + it a great fit for new contributors who want to become familiar with + the code base. It should be removed while an active PR is available, that + resolves this issue. +- *High priority:* the issue is particularly important as it can + prevent people from releasing their projects or cause data loss. +- *Needs testing*: the issue/pull request could not be completely tested + and thus need further testing. This can mean that it needs to be tested + on different hardware/software configurations or even that the steps to + reproduce are not certain. +- *Needs work*: the pull request needs additional work before it can be merged. + Also for issues that are very incomplete, such as missing reproduction steps. +- *Performance*: issues that directly impact engine or editor performance. + Can also be used for pull requests that improve performance or add low-end-friendly options. + Should not be coupled with *Usability*. +- *Regression*: the bug appeared after a stable release not exhibiting + the bug was released. +- *Salvageable*: the pull request can't be merged due to design issues or + merge conflicts and its author is not active anymore. However, it can still + be picked up by another contributor to bring it to a mergeable state. + To do so, you need to open a new pull request based on the original pull request. +- *Spam*: intentional spam issues, and extremely low-effort PRs. Used + sparingly, since we give contributors and users the benefit of the doubt. In + most cases, *Needs work* or *Archived* is more appropriate. +- *Tracker*: issue used to track other issues (like all issues related to + the plugin system). +- *Usability*: issues that directly impact user usability. Should not be coupled with *Performance*. + +The categories are used for general triage of the issues. They can be combined +in some way when relevant, e.g. an issue can be labeled *Bug* and *Usability* +at the same time if it's a bug that affects usability. Or *Enhancement* and +*Discussion* if it's an improvement that requires discussion of the best +approach. At least one of the categories *Bug*, *Enhancement*, or *Discussion* +are used to describe an issue or pull request. + +Topics: +^^^^^^^ + +- *2D*: relates to 2D nodes. Should be coupled with one of the labels + below, and should not be coupled with *3D*. +- *3D*: relates to 3D nodes. Should be coupled with one of the labels + below, and should not be coupled with *2D*. +- *Animation*: relates to the Animation system, editors and importers. +- *Assetlib*: relates to issues with the asset library. +- *Audio*: relates to the audio features (low- and high-level). +- *Buildsystem*: relates to building issues, either linked to the SCons + buildsystem or to compiler peculiarities. +- *Codestyle*: relates to the programming style used within the codebase. +- *Core*: anything related to the core engine. Specific topics are split off separately as they crop up. +- *Dotnet*: relates to the C# / .NET bindings. +- *Editor*: relates to issues in the editor (mainly UI). +- *Export*: relates to the export system and templates. +- *GDExtension*: relates to the GDExtension system for native extensions. +- *GDScript*: relates to GDScript. +- *GUI*: relates to GUI (Control) nodes or to Nodes that compose user interfaces. +- *Import*: relates to the resource import system. +- *Input*: relates to the input system. +- *I18n*: relates to internationalization. +- *Multiplayer*: relates to multiplayer (high-level networking) systems. +- *Navigation*: relates to the navigation system (including A* and navmeshes). +- *Network*: relates to (low-level) networking. +- *Particles*: particles, particle systems and their editors. +- *Physics*: relates to the physics engine (2D/3D). +- *Plugin*: relates to problems encountered while writing plugins. +- *Porting*: relates to some specific platforms or exporting projects. +- *Rendering*: relates to the 2D and 3D rendering engines. +- *Shaders*: relates to the Godot shader language or visual shaders. +- *Tests*: relates to unit tests. +- *Thirdparty*: relates to third-party libraries used in Godot. +- *XR*: relates to Augmented Reality or Virtual Reality. + +Issues would typically correspond to only one topic, though it's not +unthinkable to see issues that fit two bills. The general idea is that +there will be specialized contributors teams behind all topics, so they +can focus on the issues labelled with their team's topic. + +Platforms: +^^^^^^^^^^ + +*Android*, *iOS*, *LinuxBSD*, *macOS*, *Web*, *Windows* + +By default, it is assumed that a given issue applies to all platforms. +If one of the platform labels is used, it is then exclusive and the +previous assumption doesn't stand anymore (so if it's a bug on e.g. +Android and Linux exclusively, select those two platforms). + +Documentation labels +~~~~~~~~~~~~~~~~~~~~ + +In the `documentation repository `__, we +use the following `labels `__: + +- *Archived*: either a duplicate of another issue, or invalid. Such an + issue would also be closed. +- *Bug*: Incorrect information in an existing page. Not to be used for + *missing* information. +- *Cherrypick*: describes something that can be backported to a stable branch + after being merged in the ``master`` branch. +- *Dependencies*: describes pull requests that update a dependency file. +- *Discussion*: the issue is not consensual and needs further + discussion to define what exactly should be done to address the + topic. +- *Enhancement*: new information to be added in an existing page. +- *Good first issue*: the issue is *assumed* to be an easy one to fix, which makes + it a great fit for new contributors who want to become familiar with + the code base. It should be removed while an active PR is available, that + resolves this issue. +- *Linked demo PR*: the PR has a corresponding PR to the + `Godot Demo Projects `__ + repository which must be merged at the same time. Any changes to code in + tutorials that have a corresponding demo, such as :ref:`doc_your_first_2d_game`, + need to update both repositories so that the tutorial code stays in sync with + the completed demo. +- *Needs work*: the pull request needs additional work before it can be merged. +- *Python*: Pull requests that update Python code. +- *Salvageable*: the pull request can't be merged due to design issues or + merge conflicts and its author is not active anymore. However, it can still + be picked up by an external contributor to bring it to a mergeable state. + To do so, you need to open a new pull request based on the original pull request. +- *Tracker*: issue used to track other issues (like all issues related to + the plugin system). +- *Waiting on PR merge*: the PR documents an engine PR that has not been merged + yet. + +Area: +^^^^^ + +- *About*: Issues and PRs related to the About section of the documentation and other general articles. +- *Class reference*: the issue is about the class reference, not a documentation page. +- *Community*: Issues and PRs related to the Community section of the documentation. +- *Contributing*: Issues and PRs related to the Contributing/Development section of the documentation. +- *Getting started*: Issues and PRs related to the Getting Started section of the documentation. +- *Manual*: Issues and PRs related to the Manual/Tutorials section of the documentation. + +Content: +^^^^^^^^ + +- *Images*: Issues and PRs involving outdated or incorrect images in articles. +- *Example code*: Issues and PRs involving writing or updating code examples. +- *New page*: Issues and PRs related to creation of new documentation pages for new or undocumented features. +- *Organization*: Issues and PRs related to reorganizing the content. +- *Proofreading*: Issues and PRs related to proofreading the documentation. +- *Redirect*: Issues and PRs involving moving content and adding a redirect rule on the backend. +- *Website*: Issues related to adding website features and fixing bugs, whether on the front or back-end, + +Topic: +^^^^^^ + +The available topics describe the same content as the topics in the main +repository. + +Milestones +~~~~~~~~~~ + +`Milestones `_ are used for +some issues and all PRs. + +We have milestones for specific minor engine versions, like ``4.5`` and ``4.6``, +as well as general milestones for major engine versions, like ``3.x`` and +``4.x``. In the ``godot-proposals`` repo, we also have a ``5.0`` milestone for +compatibility-breaking changes that will be considered for Godot 5.0, in many +years. + +Issues are assigned to the current development milestone, such as ``4.5``, if +they are related to features introduced in that engine version, or are bugs +(regressions) in that version. Additionally, all issues completed during the +development of that engine version are added to the milestone, so that users can +see at a glance in which minor version an issue was first fixed. We don't always +use the ``4.x`` milestone for issues, since by default all issues are related to +Godot 4.x. However, we do use the ``3.x`` milestone to mark issues that are +specific to Godot 3.x. + +All pull requests are assigned to a milestone. By default, enhancement and +feature PRs are assigned to the ``4.x`` milestone, and bugs are assigned to the +current development milestone, such as ``4.5``. Towards the end of the minor +version's development, PRs currently in that milestone are reassessed. If +a PR is no longer being considered for that version, it is reassigned to either the +major version milestone (``4.x``), or the next minor version milestone (such as +``4.6``). + +Pull requests in the ``4.x`` milestone are reassigned to the current minor +engine version, such as ``4.5``, when the review process is complete, and the +production team decides that the PR is ready to be merged soon. Note that +this usually requires more than one approving review. + +The milestone assigned to a PR is a goal, not a guarantee. New features and +enhancements are merged when they are ready. While reviewers and maintainers do +their best to review PRs in time for the current version, at some point we reach +the beta, feature freeze, and then release; and existing PRs are reassigned to +the next minor version, or to ``4.x``. As a rule, we assign new features to the +``4.x`` milestone initially to avoid continually reassigning a PR from version +to version. However, a PR being in ``4.x`` does not mean it won't be merged; +it's just the default for new features. \ No newline at end of file