jonkeane commented on a change in pull request #12140:
URL: https://github.com/apache/arrow/pull/12140#discussion_r784952355
##########
File path: docs/source/developers/guide/step_by_step/testing.rst
##########
@@ -62,29 +62,101 @@ In this section we outline steps needed for unit testing
in Arrow.
If the tests start failing, try to recompile
PyArrow or C++.
-
+
.. note::
**Recompiling Cython**
If you only make changes to `.py` files, you do not need to
recompile PyArrow. However, you should recompile it if you make
changes in `.pyx` or `.pxd` files.
-
+
To do that run this command again:
.. code:: console
$ python setup.py build_ext --inplace
.. note::
-
+
**Recompiling C++**
Similarly, you will need to recompile the C++ code if you have
made changes to any C++ files. In this case,
- re-run the cmake commands again.
+ re-run the cmake commands again.
.. tab:: R tests
- .. TODO
+ We use `testthat <https://testthat.r-lib.org/index.html>`_ for unit
testing in R. More specifically, we use the `3rd edition of testthat
<https://testthat.r-lib.org/articles/third-edition.html>`_. On rare occasions
we might want the behaviour of the 2nd edition of testthat, which is indicated
by ``testthat::local_edition(2)``.
+
+ **Structure**
+
+ Expect the usual testthat folder structure:
+
+ .. code-block:: R
+
+ tests
+ ├── testthat # test files live here
+ └── testthat.R # runs tests when R CMD check runs (e.g. with
devtools::check())
+
+ This is the fundamental structure of testing in R with ``testthat``.
Files such as ``testthat.R`` are not expected to change very often. For the
``arrow`` R package ``testthat.R`` also defines how the results of the various
tests are displayed / reported in the console.
+
+ Usually, most files in the ``R/`` sub-folder have a corresponding test
file in ``tests/testthat``.
+
+ **Running tests**
+
+ To run all tests in a package locally call
+
+ .. code-block:: R
+
+ devtools::test()
+
+ in the R console. Alternatively, you can use
+
+ .. code:: console
+
+ $ make test
+
+ in the shell.
+
+ You can run the tests in a single test file you have open with
+
+ .. code-block:: R
+
+ devtools::test_active_file()
+
+ All tests are also run as part of our continuous integration (CI)
pipelines.
+
+ The Arrow R Developer guide also has a section on running tests. You can
check it out `here
<https://arrow.apache.org/docs/r/articles/developing.html#running-tests>`_.
Review comment:
I think I got the RST correct here, but not sure about that `_` at the
end, that seems out of place, but I'm no RST expert
##########
File path: docs/source/developers/guide/step_by_step/testing.rst
##########
@@ -62,29 +62,101 @@ In this section we outline steps needed for unit testing
in Arrow.
If the tests start failing, try to recompile
PyArrow or C++.
-
+
.. note::
**Recompiling Cython**
If you only make changes to `.py` files, you do not need to
recompile PyArrow. However, you should recompile it if you make
changes in `.pyx` or `.pxd` files.
-
+
To do that run this command again:
.. code:: console
$ python setup.py build_ext --inplace
.. note::
-
+
**Recompiling C++**
Similarly, you will need to recompile the C++ code if you have
made changes to any C++ files. In this case,
- re-run the cmake commands again.
+ re-run the cmake commands again.
.. tab:: R tests
- .. TODO
+ We use `testthat <https://testthat.r-lib.org/index.html>`_ for unit
testing in R. More specifically, we use the `3rd edition of testthat
<https://testthat.r-lib.org/articles/third-edition.html>`_. On rare occasions
we might want the behaviour of the 2nd edition of testthat, which is indicated
by ``testthat::local_edition(2)``.
+
+ **Structure**
+
+ Expect the usual testthat folder structure:
+
+ .. code-block:: R
+
+ tests
+ ├── testthat # test files live here
+ └── testthat.R # runs tests when R CMD check runs (e.g. with
devtools::check())
+
+ This is the fundamental structure of testing in R with ``testthat``.
Files such as ``testthat.R`` are not expected to change very often. For the
``arrow`` R package ``testthat.R`` also defines how the results of the various
tests are displayed / reported in the console.
+
+ Usually, most files in the ``R/`` sub-folder have a corresponding test
file in ``tests/testthat``.
+
+ **Running tests**
+
+ To run all tests in a package locally call
+
+ .. code-block:: R
+
+ devtools::test()
+
+ in the R console. Alternatively, you can use
+
+ .. code:: console
+
+ $ make test
+
+ in the shell.
+
+ You can run the tests in a single test file you have open with
+
+ .. code-block:: R
+
+ devtools::test_active_file()
+
+ All tests are also run as part of our continuous integration (CI)
pipelines.
+
+ The Arrow R Developer guide also has a section on running tests. You can
check it out `here
<https://arrow.apache.org/docs/r/articles/developing.html#running-tests>`_.
+
+ **Good practice**
+
+ In general any change to source code needs to be accompanied by unit
tests. All tests are expected to pass before a pull request is merged.
+
+ * Add functionality -> add unit tests
+ * Modify functionality -> update unit tests
+ * Solve a bug -> add unit test before solving it, which helps prove the
bug and its fix
+ * Performance improvements should be reflected in benchmarks (which are
also tests)
+ * An exception could be refactoring functionality that is fully covered
by unit tests
+
+ If the new functionality is a user-facing or API change, you will almost
certainly need to change tests — if no tests need to be changed it might mean
the tests aren't right! If the new functionality is a refactor and no APIs are
changing, there might not need to be test changes.
Review comment:
```suggestion
A good rule of thumb is: If the new functionality is a user-facing or
API change, you will almost certainly need to change tests — if no tests need
to be changed it might mean the tests aren't right! If the new functionality is
a refactor and no APIs are changing, there might not need to be test changes.
```
##########
File path: docs/source/developers/guide/step_by_step/testing.rst
##########
@@ -62,29 +62,101 @@ In this section we outline steps needed for unit testing
in Arrow.
If the tests start failing, try to recompile
PyArrow or C++.
-
+
.. note::
**Recompiling Cython**
If you only make changes to `.py` files, you do not need to
recompile PyArrow. However, you should recompile it if you make
changes in `.pyx` or `.pxd` files.
-
+
To do that run this command again:
.. code:: console
$ python setup.py build_ext --inplace
.. note::
-
+
**Recompiling C++**
Similarly, you will need to recompile the C++ code if you have
made changes to any C++ files. In this case,
- re-run the cmake commands again.
+ re-run the cmake commands again.
.. tab:: R tests
- .. TODO
+ We use `testthat <https://testthat.r-lib.org/index.html>`_ for unit
testing in R. More specifically, we use the `3rd edition of testthat
<https://testthat.r-lib.org/articles/third-edition.html>`_. On rare occasions
we might want the behaviour of the 2nd edition of testthat, which is indicated
by ``testthat::local_edition(2)``.
+
+ **Structure**
+
+ Expect the usual testthat folder structure:
+
+ .. code-block:: R
+
+ tests
+ ├── testthat # test files live here
+ └── testthat.R # runs tests when R CMD check runs (e.g. with
devtools::check())
+
+ This is the fundamental structure of testing in R with ``testthat``.
Files such as ``testthat.R`` are not expected to change very often. For the
``arrow`` R package ``testthat.R`` also defines how the results of the various
tests are displayed / reported in the console.
+
+ Usually, most files in the ``R/`` sub-folder have a corresponding test
file in ``tests/testthat``.
+
+ **Running tests**
+
+ To run all tests in a package locally call
+
+ .. code-block:: R
+
+ devtools::test()
+
+ in the R console. Alternatively, you can use
+
+ .. code:: console
+
+ $ make test
+
+ in the shell.
+
+ You can run the tests in a single test file you have open with
+
+ .. code-block:: R
+
+ devtools::test_active_file()
+
+ All tests are also run as part of our continuous integration (CI)
pipelines.
+
+ The Arrow R Developer guide also has a section on running tests. You can
check it out `here
<https://arrow.apache.org/docs/r/articles/developing.html#running-tests>`_.
Review comment:
```suggestion
The `Arrow R Developer guide also has a
section<https://arrow.apache.org/docs/r/articles/developing.html#running-tests>`_
on running tests.
```
Avoids the click here type of pattern (and is a bit more concise.
##########
File path: docs/source/developers/guide/step_by_step/testing.rst
##########
@@ -62,29 +62,101 @@ In this section we outline steps needed for unit testing
in Arrow.
If the tests start failing, try to recompile
PyArrow or C++.
-
+
.. note::
**Recompiling Cython**
If you only make changes to `.py` files, you do not need to
recompile PyArrow. However, you should recompile it if you make
changes in `.pyx` or `.pxd` files.
-
+
To do that run this command again:
.. code:: console
$ python setup.py build_ext --inplace
.. note::
-
+
**Recompiling C++**
Similarly, you will need to recompile the C++ code if you have
made changes to any C++ files. In this case,
- re-run the cmake commands again.
+ re-run the cmake commands again.
.. tab:: R tests
- .. TODO
+ We use `testthat <https://testthat.r-lib.org/index.html>`_ for unit
testing in R. More specifically, we use the `3rd edition of testthat
<https://testthat.r-lib.org/articles/third-edition.html>`_. On rare occasions
we might want the behaviour of the 2nd edition of testthat, which is indicated
by ``testthat::local_edition(2)``.
+
+ **Structure**
+
+ Expect the usual testthat folder structure:
+
+ .. code-block:: R
+
+ tests
+ ├── testthat # test files live here
+ └── testthat.R # runs tests when R CMD check runs (e.g. with
devtools::check())
+
+ This is the fundamental structure of testing in R with ``testthat``.
Files such as ``testthat.R`` are not expected to change very often. For the
``arrow`` R package ``testthat.R`` also defines how the results of the various
tests are displayed / reported in the console.
+
+ Usually, most files in the ``R/`` sub-folder have a corresponding test
file in ``tests/testthat``.
+
+ **Running tests**
+
+ To run all tests in a package locally call
+
+ .. code-block:: R
+
+ devtools::test()
+
+ in the R console. Alternatively, you can use
+
+ .. code:: console
+
+ $ make test
+
+ in the shell.
+
+ You can run the tests in a single test file you have open with
+
+ .. code-block:: R
+
+ devtools::test_active_file()
+
+ All tests are also run as part of our continuous integration (CI)
pipelines.
+
+ The Arrow R Developer guide also has a section on running tests. You can
check it out `here
<https://arrow.apache.org/docs/r/articles/developing.html#running-tests>`_.
+
+ **Good practice**
+
+ In general any change to source code needs to be accompanied by unit
tests. All tests are expected to pass before a pull request is merged.
+
+ * Add functionality -> add unit tests
+ * Modify functionality -> update unit tests
+ * Solve a bug -> add unit test before solving it, which helps prove the
bug and its fix
+ * Performance improvements should be reflected in benchmarks (which are
also tests)
+ * An exception could be refactoring functionality that is fully covered
by unit tests
+
+ If the new functionality is a user-facing or API change, you will almost
certainly need to change tests — if no tests need to be changed it might mean
the tests aren't right! If the new functionality is a refactor and no APIs are
changing, there might not need to be test changes.
+
+ **Testing helpers**
+
+ To complement the ``testthat`` functionality, the ``arrow`` R package
has defined a series of specific utility functions (called helpers), such as:
+
+ * expectations - these start with ``expect_`` and are used to compare
objects
+ - for example, ``expect_altrep_roundtrip()`` compares the result
of a function ``fn`` run on a vector ``x`` with the result of the same function
run on the altrep version of ``x``. More generally, ``expect_…_roundtrip()``
functions take an input, convert it to some other format (e.g. arrow) and then
convert it back, confirming that the values are the same.
+ * ``skip_`` - skips a unit test - think of them as acceptable fails.
Situations in which we might want to skip unit tests:
+
+ - ``skip_if_r_version()`` - this is a specific ``arrow`` skip. For
example, we use this to skip a unit test when the R version is 3.5.0 and below
(``skip_if_r_version(“3.5.0”)``). You will likely see it used when the
functionality we are testing depends on features introduced after version 3.5.0
of R (such as the alternative representation of vectors, Altrep, introduced in
R 3.5.0, but with significant additions in subsequent releases). As part of our
CI workflow we test against different versions of R and this is where this
feature comes in.
+ - ``skip_if_not_available()`` - another specific {arrow} skip. Arrow
(libarrow) has a series of additional features that can be switched on or off
(but this needs to happen at build time). If a unit test depends on such a
feature and this feature is not available (i.e. was not selected when libarrow
was built) the test is skipped, as opposed to having a failed test.
Review comment:
```suggestion
- ``skip_if_not_available()`` - another specific {arrow} skip. Arrow
(libarrow) has a number of optional features that can be switched on or off
(which happens at build time). If a unit test depends on such a feature and
this feature is not available (i.e. was not selected when libarrow was built)
the test is skipped, as opposed to having a failed test.
```
##########
File path: docs/source/developers/guide/step_by_step/testing.rst
##########
@@ -62,29 +62,101 @@ In this section we outline steps needed for unit testing
in Arrow.
If the tests start failing, try to recompile
PyArrow or C++.
-
+
.. note::
**Recompiling Cython**
If you only make changes to `.py` files, you do not need to
recompile PyArrow. However, you should recompile it if you make
changes in `.pyx` or `.pxd` files.
-
+
To do that run this command again:
.. code:: console
$ python setup.py build_ext --inplace
.. note::
-
+
**Recompiling C++**
Similarly, you will need to recompile the C++ code if you have
made changes to any C++ files. In this case,
- re-run the cmake commands again.
+ re-run the cmake commands again.
.. tab:: R tests
- .. TODO
+ We use `testthat <https://testthat.r-lib.org/index.html>`_ for unit
testing in R. More specifically, we use the `3rd edition of testthat
<https://testthat.r-lib.org/articles/third-edition.html>`_. On rare occasions
we might want the behaviour of the 2nd edition of testthat, which is indicated
by ``testthat::local_edition(2)``.
+
+ **Structure**
+
+ Expect the usual testthat folder structure:
+
+ .. code-block:: R
+
+ tests
+ ├── testthat # test files live here
+ └── testthat.R # runs tests when R CMD check runs (e.g. with
devtools::check())
+
+ This is the fundamental structure of testing in R with ``testthat``.
Files such as ``testthat.R`` are not expected to change very often. For the
``arrow`` R package ``testthat.R`` also defines how the results of the various
tests are displayed / reported in the console.
+
+ Usually, most files in the ``R/`` sub-folder have a corresponding test
file in ``tests/testthat``.
+
+ **Running tests**
+
+ To run all tests in a package locally call
+
+ .. code-block:: R
+
+ devtools::test()
+
+ in the R console. Alternatively, you can use
+
+ .. code:: console
+
+ $ make test
+
+ in the shell.
+
+ You can run the tests in a single test file you have open with
+
+ .. code-block:: R
+
+ devtools::test_active_file()
+
+ All tests are also run as part of our continuous integration (CI)
pipelines.
+
+ The Arrow R Developer guide also has a section on running tests. You can
check it out `here
<https://arrow.apache.org/docs/r/articles/developing.html#running-tests>`_.
+
+ **Good practice**
+
+ In general any change to source code needs to be accompanied by unit
tests. All tests are expected to pass before a pull request is merged.
+
+ * Add functionality -> add unit tests
+ * Modify functionality -> update unit tests
+ * Solve a bug -> add unit test before solving it, which helps prove the
bug and its fix
+ * Performance improvements should be reflected in benchmarks (which are
also tests)
+ * An exception could be refactoring functionality that is fully covered
by unit tests
+
+ If the new functionality is a user-facing or API change, you will almost
certainly need to change tests — if no tests need to be changed it might mean
the tests aren't right! If the new functionality is a refactor and no APIs are
changing, there might not need to be test changes.
+
+ **Testing helpers**
+
+ To complement the ``testthat`` functionality, the ``arrow`` R package
has defined a series of specific utility functions (called helpers), such as:
+
+ * expectations - these start with ``expect_`` and are used to compare
objects
+ - for example, ``expect_altrep_roundtrip()`` compares the result
of a function ``fn`` run on a vector ``x`` with the result of the same function
run on the altrep version of ``x``. More generally, ``expect_…_roundtrip()``
functions take an input, convert it to some other format (e.g. arrow) and then
convert it back, confirming that the values are the same.
Review comment:
I wonder if we should also provide an example here that shows a call?
The two function arguments feel a bit out of place / floating out there: either
we could drop them, or we should show them in use in `expect_altrep_roundtrip()`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
