[libcxx-commits] [libcxx] Update testing documentation with testing guidelines. (PR #87928)
via libcxx-commits
libcxx-commits at lists.llvm.org
Thu Jun 13 07:25:16 PDT 2024
https://github.com/EricWF updated https://github.com/llvm/llvm-project/pull/87928
>From 259571a7c6922db8dd413503a1ea570c411fd3cb Mon Sep 17 00:00:00 2001
From: Eric Fiselier <eric at efcs.ca>
Date: Sun, 7 Apr 2024 12:37:29 -0400
Subject: [PATCH 1/5] Update testing documentation with testing guidelines.
The new documentation intends to lay out _general considerations_
for writing tests, rather than absolute rules, under the understanding
that every libc++ engineer is talented and knowledgable.
It's meant as a starting point for discussion.
Please critique away!
---
libcxx/docs/TestingLibcxx.rst | 36 +++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/libcxx/docs/TestingLibcxx.rst b/libcxx/docs/TestingLibcxx.rst
index 50ee9d4ee400b..b1dd4bf7a3977 100644
--- a/libcxx/docs/TestingLibcxx.rst
+++ b/libcxx/docs/TestingLibcxx.rst
@@ -180,6 +180,42 @@ The tests of libc++ are stored in libc++'s testing related subdirectories:
``libcxx/test/libcxx``. The structure of this directories follows the
structure of ``libcxx/test/std``.
+Principles of testing
+---------------------
+
+Tests are a practical way to validate the correctness of the code. As such, they contain pragmatic trade offs between
+the cost of writing and maintaining the tests and the value they provide. Please consider the following principles when
+writing tests:
+
+- **Consider the next reader**
+
+ Tests should be obvious to tho future reader. Avoid too much boiler plate or other
+ distractions. Ensure each test has enough context to understand what it is testing. Avoid gratuitous use of advanced
+ test features or abstractions.
+
+- **Consider the effect of time**
+
+ Tests should be resilient to the effects of time. Tests are not static; They are
+ living documents that change over time. The intent of a test can become less clear over time.
+
+- **Consider the edge cases carefully**
+
+ Undefined behavior and edge cases are often the source of bugs. Tests should exercise these cases to ensure that
+ the code under test behaves correctly. It's important to write tests for the easy cases as well as the hard ones.
+
+- **Consider the focus**
+
+ Each test case should test a single concern. Ideally a test should only fail when this concern
+ is violated. Focused tests are not flakey. If a test case covers multiple concerns, consider splitting it into multiple
+ test cases.
+
+
+Note that these are principles, not rules. There are cases where it is appropriate to break these principles.
+During code review, if you feel that a test is not following these principles, please discuss it with the author.
+
+Remember code review is a **consensus building** process, not a **gatekeeping** process. Your fellow engineers are
+all working towards the same goal: to make the code better.
+
Structure of a test
-------------------
>From 0fcb8b721f180542c261994e8893c8f022826245 Mon Sep 17 00:00:00 2001
From: Eric <eric at efcs.ca>
Date: Thu, 11 Apr 2024 10:19:27 -0400
Subject: [PATCH 2/5] Update libcxx/docs/TestingLibcxx.rst
Co-authored-by: Hristo Hristov <zingam at outlook.com>
---
libcxx/docs/TestingLibcxx.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libcxx/docs/TestingLibcxx.rst b/libcxx/docs/TestingLibcxx.rst
index b1dd4bf7a3977..2d47419c17f05 100644
--- a/libcxx/docs/TestingLibcxx.rst
+++ b/libcxx/docs/TestingLibcxx.rst
@@ -189,7 +189,7 @@ writing tests:
- **Consider the next reader**
- Tests should be obvious to tho future reader. Avoid too much boiler plate or other
+ Tests should be obvious to the future reader. Avoid too much boiler plate or other
distractions. Ensure each test has enough context to understand what it is testing. Avoid gratuitous use of advanced
test features or abstractions.
>From e28f122562eaa6522e18447ae71cd83d43557b93 Mon Sep 17 00:00:00 2001
From: Eric <eric at efcs.ca>
Date: Thu, 11 Apr 2024 14:44:04 -0400
Subject: [PATCH 3/5] Update TestingLibcxx.rst
---
libcxx/docs/TestingLibcxx.rst | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/libcxx/docs/TestingLibcxx.rst b/libcxx/docs/TestingLibcxx.rst
index 2d47419c17f05..6414b2595b429 100644
--- a/libcxx/docs/TestingLibcxx.rst
+++ b/libcxx/docs/TestingLibcxx.rst
@@ -196,25 +196,21 @@ writing tests:
- **Consider the effect of time**
Tests should be resilient to the effects of time. Tests are not static; They are
- living documents that change over time. The intent of a test can become less clear over time.
+ living documents that change. As they change the original intent of a test can become less clear.
- **Consider the edge cases carefully**
Undefined behavior and edge cases are often the source of bugs. Tests should exercise these cases to ensure that
the code under test behaves correctly. It's important to write tests for the easy cases as well as the hard ones.
+ When security is a concern, write fuzz tests.
- **Consider the focus**
Each test case should test a single concern. Ideally a test should only fail when this concern
- is violated. Focused tests are not flakey. If a test case covers multiple concerns, consider splitting it into multiple
+ is violated. Focused tests are not flaky. If a test case covers multiple concerns, consider splitting it into multiple
test cases.
-
Note that these are principles, not rules. There are cases where it is appropriate to break these principles.
-During code review, if you feel that a test is not following these principles, please discuss it with the author.
-
-Remember code review is a **consensus building** process, not a **gatekeeping** process. Your fellow engineers are
-all working towards the same goal: to make the code better.
Structure of a test
-------------------
>From e24544c8f588a13fad389795279577253c8f5447 Mon Sep 17 00:00:00 2001
From: Eric <eric at efcs.ca>
Date: Thu, 13 Jun 2024 07:56:44 -0400
Subject: [PATCH 4/5] Update TestingLibcxx.rst
* Add comment about comments in tests as requested by Mark and Louis.
* Relocate section higher in document as suggested by ?ouis.
* Re-add section about consensus building, but without mention of code review.
---
libcxx/docs/TestingLibcxx.rst | 55 ++++++++++++++++++++---------------
1 file changed, 31 insertions(+), 24 deletions(-)
diff --git a/libcxx/docs/TestingLibcxx.rst b/libcxx/docs/TestingLibcxx.rst
index 6414b2595b429..d45b886860bf7 100644
--- a/libcxx/docs/TestingLibcxx.rst
+++ b/libcxx/docs/TestingLibcxx.rst
@@ -155,30 +155,6 @@ few requirements to the test suite. Here's some stuff you should know:
necessarily available on all devices we may want to run the tests on (even
though supporting Python is probably trivial for the build-host).
-Structure of the testing related directories
---------------------------------------------
-
-The tests of libc++ are stored in libc++'s testing related subdirectories:
-
-- ``libcxx/test/support`` This directory contains several helper headers with
- generic parts for the tests. The most important header is ``test_macros.h``.
- This file contains configuration information regarding the platform used.
- This is similar to the ``__config`` file in libc++'s ``include`` directory.
- Since libc++'s tests are used by other Standard libraries, tests should use
- the ``TEST_FOO`` macros instead of the ``_LIBCPP_FOO`` macros, which are
- specific to libc++.
-- ``libcxx/test/std`` This directory contains the tests that validate the library under
- test conforms to the C++ Standard. The paths and the names of the test match
- the section names in the C++ Standard. Note that the C++ Standard sometimes
- reorganises its structure, therefore some tests are at a location based on
- where they appeared historically in the standard. We try to strike a balance
- between keeping things at up-to-date locations and unnecessary churn.
-- ``libcxx/test/libcxx`` This directory contains the tests that validate libc++
- specific behavior and implementation details. For example, libc++ has
- "wrapped iterators" that perform bounds checks. Since those are specific to
- libc++ and not mandated by the Standard, tests for those are located under
- ``libcxx/test/libcxx``. The structure of this directories follows the
- structure of ``libcxx/test/std``.
Principles of testing
---------------------
@@ -193,6 +169,8 @@ writing tests:
distractions. Ensure each test has enough context to understand what it is testing. Avoid gratuitous use of advanced
test features or abstractions.
+ We strongly encourage the intent of a test and the reason for its existence to be documented in plain text comments.
+
- **Consider the effect of time**
Tests should be resilient to the effects of time. Tests are not static; They are
@@ -212,6 +190,35 @@ writing tests:
Note that these are principles, not rules. There are cases where it is appropriate to break these principles.
+These principles should be used **to build consensus**, not to **gatekeep**. Your fellow engineers are
+all working towards the same goal: to make the code better.
+
+Structure of the testing related directories
+--------------------------------------------
+
+The tests of libc++ are stored in libc++'s testing related subdirectories:
+
+- ``libcxx/test/support`` This directory contains several helper headers with
+ generic parts for the tests. The most important header is ``test_macros.h``.
+ This file contains configuration information regarding the platform used.
+ This is similar to the ``__config`` file in libc++'s ``include`` directory.
+ Since libc++'s tests are used by other Standard libraries, tests should use
+ the ``TEST_FOO`` macros instead of the ``_LIBCPP_FOO`` macros, which are
+ specific to libc++.
+- ``libcxx/test/std`` This directory contains the tests that validate the library under
+ test conforms to the C++ Standard. The paths and the names of the test match
+ the section names in the C++ Standard. Note that the C++ Standard sometimes
+ reorganises its structure, therefore some tests are at a location based on
+ where they appeared historically in the standard. We try to strike a balance
+ between keeping things at up-to-date locations and unnecessary churn.
+- ``libcxx/test/libcxx`` This directory contains the tests that validate libc++
+ specific behavior and implementation details. For example, libc++ has
+ "wrapped iterators" that perform bounds checks. Since those are specific to
+ libc++ and not mandated by the Standard, tests for those are located under
+ ``libcxx/test/libcxx``. The structure of this directories follows the
+ structure of ``libcxx/test/std``.
+
+
Structure of a test
-------------------
>From 35b52e849ff66a0109c098413182494eb9cbdb15 Mon Sep 17 00:00:00 2001
From: Eric <eric at efcs.ca>
Date: Thu, 13 Jun 2024 07:57:42 -0400
Subject: [PATCH 5/5] Update TestingLibcxx.rst
Minor grammar fix.
---
libcxx/docs/TestingLibcxx.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libcxx/docs/TestingLibcxx.rst b/libcxx/docs/TestingLibcxx.rst
index d45b886860bf7..ced0c50d3059c 100644
--- a/libcxx/docs/TestingLibcxx.rst
+++ b/libcxx/docs/TestingLibcxx.rst
@@ -190,7 +190,7 @@ writing tests:
Note that these are principles, not rules. There are cases where it is appropriate to break these principles.
-These principles should be used **to build consensus**, not to **gatekeep**. Your fellow engineers are
+These principles should be used **to build consensus** and not to **gatekeep**. Your fellow engineers are
all working towards the same goal: to make the code better.
Structure of the testing related directories
More information about the libcxx-commits
mailing list