Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update unittesting.rst, added section "Test report" #9250

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Update unittesting.rst, added section "Test report" #9250

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions docs/developers_guide/unittesting.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,22 @@ antialiasing or font differences), the script
:source:`parse_dash_results.py <scripts/parse_dash_results.py>`
can help you when you are updating the local test masks.


Test report
=======================================

When running the unit tests simply using ``make test`` or ``ctest`` three kinds of output are generated.

A few summary data files are written to ``build/Testing/Temporary``, in particular ``LastTest.log`` and ``LastTestsFailed.log``. These will be overwritten every time you call ``make test`` or `ctest``.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When it comes to files path, please use dedicated rst role, I.e

Suggested change
A few summary data files are written to ``build/Testing/Temporary``, in particular ``LastTest.log`` and ``LastTestsFailed.log``. These will be overwritten every time you call ``make test`` or `ctest``.
A few summary data files are written to :file:`build/Testing/Temporary`, in particular :file:`LastTest.log` and :file:`LastTestsFailed.log`. These will be overwritten every time you call ``make test`` or ``ctest``.

Some more occurrences below


A bunch of temporary files are saved to ``$TMPDIR``, and if the tests are written as intended, almost all of this is deleted in the teardown process, at the end of each test item. If any of this data is not cleaned up, by default your OS will eventually clean up all files in ``$TMPDIR`` after some time has expired.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this paragraph needs to be included. It's not really relevant to running the tests

Copy link
Author

@velle velle Sep 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was surprised to see that all (or almost all) files created during test was deleted during tear down. I don't know if that is the standard way, but I would have expected them to be left in place, so one can inspect the results; at the very least for the failed tests.

So I wrote this paragraph to help the reader understand that he won't be able to find such files; not even for failed tests. (1)

I must admit, that I am not even sure that my assumption above is correct; ie the assumption that even for a failed test the temporary files are deleted during tear down (1). Can you confirm? Is there a rule or guideline that a unit test should delete all files in tear down?

(1): Except for a few special cases where a few relevant files are saved to $QGIS_TEST_REPORT.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's too much hassle to answer these questions now, then don't. I accept your changes :) If I an answer to these questions any time soon, I might suggest that as a separate PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was surprised to see that all (or almost all) files created during test was deleted during tear down. I don't know if that is the standard way, but I would have expected them to be left in place, so one can inspect the results; at the very least for the failed tests.

I must admit, that I am not even sure that my assumption above is correct; ie the assumption that even for a failed test the temporary files are deleted during tear down (1). Can you confirm? Is there a rule or guideline that a unit test should delete all files in tear down?

There's no particular guideline. I guess it's more that a well designed test would give enough feedback in the test failure itself to diagnose the error.

🤷‍♂️

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
A bunch of temporary files are saved to ``$TMPDIR``, and if the tests are written as intended, almost all of this is deleted in the teardown process, at the end of each test item. If any of this data is not cleaned up, by default your OS will eventually clean up all files in ``$TMPDIR`` after some time has expired.
A bunch of temporary files are saved to ``$TMPDIR``, your OS will eventually clean up all files in this directory after some time has expired or after reboot (depends on OS).


Some tests (either always or only in the case of a fail) will leave some some data behind, that helps the developer understand what failed. This is by default saved to ``$TMPDIR/qgis_test_report``, but the destination can be overwritten with the environment variable ``$QGIS_TEST_REPORT``.

Some of the content written to the ``$QGIS_TEST_REPORT`` folder are html pages, and the unit test code will ask the OS to open these pages in the default browser. On some OS, e.g. Ubuntu since v. 21.04, the default browsers are installed as snap apps, which mean they can't access the typical destinations, either ``/tmp`` or ``~/.tmp``. The user will experience a number of tabs open in the browser, but each showing an error message with access denied instead of the actual html. In this case, we recommend that you either change ``$TMPDIR`` or you you define ``$QGIS_TEST_REPORT``, e.g.
to ``~/qgis_test_report``.
Comment on lines +467 to +468
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Some of the content written to the ``$QGIS_TEST_REPORT`` folder are html pages, and the unit test code will ask the OS to open these pages in the default browser. On some OS, e.g. Ubuntu since v. 21.04, the default browsers are installed as snap apps, which mean they can't access the typical destinations, either ``/tmp`` or ``~/.tmp``. The user will experience a number of tabs open in the browser, but each showing an error message with access denied instead of the actual html. In this case, we recommend that you either change ``$TMPDIR`` or you you define ``$QGIS_TEST_REPORT``, e.g.
to ``~/qgis_test_report``.
Some of the content written to the ``$QGIS_TEST_REPORT`` folder are html pages, and the unit test code will ask the OS to open these pages in the default browser. On some OS, e.g. Ubuntu since v. 21.04, the default browsers are installed as snap apps, which mean they can't access the typical destinations, either :file:`/tmp` or :file:`~/.tmp`. The user will experience a number of tabs open in the browser, but each showing an error message with access denied instead of the actual html. In this case, we recommend that you at least change the ``$TMPDIR`` env var to a read/write permitted folder (e.g. :file:`~/my_qgis_tmp/`).

If the default /tmp is access denied then just changing the $QGIS_TEST_REPORT env var will provide html content referencing the access denied folder /tmp. The html content will be half useful ==> this need to be tested!

Copy link
Author

@velle velle Sep 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the default /tmp is access denied then just changing the $QGIS_TEST_REPORT env var will provide html content referencing the access denied folder /tmp. The html content will be half useful ==> this need to be tested!

The code PR has already been merged. I have been using it for a few days, and I have not yet noticed any html in $QGIS_TEST_REPORT that referenced anything outside $QGIS_TEST_REPORT.

CORRECTION: All images are copied to $QGIS_TEST_REPORT. And the Javascript comparator, and the img tags use this path. But some of the hyperlinks in the text body point to the original file, outside $QGIS_TEST_REPORT. That can easily be fixed, but it might take a few days before I have time. Screenshot, with doodles: https://imgur.com/a/5HhBALG

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!



Adding your unit test to CMakeLists.txt
=======================================

Expand Down
Loading