summaryrefslogtreecommitdiffstats
path: root/doc/sphinx/Pacemaker_Development/helpers.rst
blob: 6bd19266b4aa232c6a6db67d412e296886cf154a (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486
487
488
489
490
491
492
493
494
495
496
497
498
499
500
501
502
503
504
505
506
507
508
509
510
511
512
513
514
515
516
517
518
519
520
C Development Helpers
---------------------

.. index::
   single: unit testing

Refactoring
###########

Pacemaker uses an optional tool called `coccinelle <https://coccinelle.gitlabpages.inria.fr/website/>`_
to do automatic refactoring.  coccinelle is a very complicated tool that can be
difficult to understand, and the existing documentation makes it pretty tough
to get started.  Much of the documentation is either aimed at kernel developers
or takes the form of grammars.

However, it can apply very complex transformations across an entire source tree.
This is useful for tasks like code refactoring, changing APIs (number or type of
arguments, etc.), catching functions that should not be called, and changing
existing patterns.

coccinelle is driven by input scripts called `semantic patches <https://coccinelle.gitlabpages.inria.fr/website/docs/index.html>`_
written in its own language.  These scripts bear a passing resemblance to source
code patches and tell coccinelle how to match and modify a piece of source
code.  They are stored in ``devel/coccinelle`` and each script either contains
a single source transformation or several related transformations.  In general,
we try to keep these as simple as possible.

In Pacemaker development, we use a couple targets in ``devel/Makefile.am`` to
control coccinelle.  The ``cocci`` target tries to apply each script to every
Pacemaker source file, printing out any changes it would make to the console.
The ``cocci-inplace`` target does the same but also makes those changes to the
source files.  A variety of warnings might also be printed.  If you aren't working
on a new script, these can usually be ignored.

If you are working on a new coccinelle script, it can be useful (and faster) to
skip everything else and only run the new script.  The ``COCCI_FILES`` variable
can be used for this:

.. code-block:: none

   $ make -C devel COCCI_FILES=coccinelle/new-file.cocci cocci

This variable is also used for preventing some coccinelle scripts in the Pacemaker
source tree from running.  Some scripts are disabled because they are not currently
fully working or because they are there as templates.  When adding a new script,
remember to add it to this variable if it should always be run.

One complication when writing coccinelle scripts is that certain Pacemaker source
files may not use private functions (those whose name starts with ``pcmk__``).
Handling this requires work in both the Makefile and in the coccinelle scripts.

The Makefile deals with this by maintaining two lists of source files: those that
may use private functions and those that may not.  For those that may, a special
argument (``-D internal``) is added to the coccinelle command line.  This creates
a virtual dependency named ``internal``.

In the coccinelle scripts, those transformations that modify source code to use
a private function also have a dependency on ``internal``.  If that dependency
was given on the command line, the transformation will be run.  Otherwise, it will
be skipped.

This means that not all instances of an older style of code will be changed after
running a given transformation.  Some developer intervention is still necessary
to know whether a source code block should have been changed or not.

Probably the easiest way to learn how to use coccinelle is by following other
people's scripts.  In addition to the ones in the Pacemaker source directory,
there's several others on the `coccinelle website <https://coccinelle.gitlabpages.inria.fr/website/rules/>`_.

Sanitizers
##########

gcc supports a variety of run-time checks called sanitizers.  These can be used to
catch programming errors with memory, race conditions, various undefined behavior
conditions, and more.  Because these are run-time checks, they should only be used
during development and not in compiled packages or production code.

Certain sanitizers cannot be combined with others because their run-time checks
cause interfere.  Instead of trying to figure out which combinations work, it is
simplest to just enable one at a time.

Each supported sanitizer requires an installed libray.  In addition to just
enabling the sanitizer, their use can be configured with environment variables.
For example:

.. code-block:: none

   $ ASAN_OPTIONS=verbosity=1:replace_str=true crm_mon -1R

Pacemaker supports the following subset of gcc's sanitizers:

+--------------------+-------------------------+----------+----------------------+
| Sanitizer          | Configure Option        | Library  | Environment Variable |
+====================+=========================+==========+======================+
| Address            | --with-sanitizers=asan  | libasan  | ASAN_OPTIONS         |
+--------------------+-------------------------+----------+----------------------+
| Threads            | --with-sanitizers=tsan  | libtsan  | TSAN_OPTIONS         |
+--------------------+-------------------------+----------+----------------------+
| Undefined behavior | --with-sanitizers=ubsan | libubsan | UBSAN_OPTIONS        |
+--------------------+-------------------------+----------+----------------------+

The undefined behavior sanitizer further supports suboptions that need to be
given as CFLAGS when configuring pacemaker:

.. code-block:: none

   $ CFLAGS=-fsanitize=integer-divide-by-zero ./configure --with-sanitizers=ubsan

For more information, see the `gcc documentation <https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html>`_
which also provides links to more information on each sanitizer.

Unit Testing
############

Where possible, changes to the C side of Pacemaker should be accompanied by unit
tests.  Much of Pacemaker cannot effectively be unit tested (and there are other
testing systems used for those parts), but the ``lib`` subdirectory is pretty easy
to write tests for.

Pacemaker uses the `cmocka unit testing framework <https://cmocka.org/>`_ which looks
a lot like other unit testing frameworks for C and should be fairly familiar.  In
addition to regular unit tests, cmocka also gives us the ability to use
`mock functions <https://en.wikipedia.org/wiki/Mock_object>`_ for unit testing
functions that would otherwise be difficult to test.

Organization
____________

Pay close attention to the organization and naming of test cases to ensure the
unit tests continue to work as they should.

Tests are spread throughout the source tree, alongside the source code they test.
For instance, all the tests for the source code in ``lib/common/`` are in the
``lib/common/tests`` directory.  If there is no ``tests`` subdirectory, there are no
tests for that library yet.

Under that directory, there is a ``Makefile.am`` and additional subdirectories.  Each
subdirectory contains the tests for a single library source file.  For instance,
all the tests for ``lib/common/strings.c`` are in the ``lib/common/tests/strings``
directory.  Note that the test subdirectory does not have a ``.c`` suffix.  If there
is no test subdirectory, there are no tests for that file yet.

Finally, under that directory, there is a ``Makefile.am`` and then various source
files.  Each of these source files tests the single function that it is named
after.  For instance, ``lib/common/tests/strings/pcmk__btoa_test.c`` tests the
``pcmk__btoa()`` function in ``lib/common/strings.c``.  If there is no test
source file, there are no tests for that function yet.

The ``_test`` suffix on the test source file is important.  All tests have this
suffix, which means all the compiled test cases will also end with this suffix.
That lets us ignore all the compiled tests with a single line in ``.gitignore``:

.. code-block:: none

   /lib/*/tests/*/*_test

Adding a test
_____________

Testing a new function in an already testable source file
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Follow these steps if you want to test a function in a source file where there
are already other tested functions.  For the purposes of this example, we will
add a test for the ``pcmk__scan_port()`` function in ``lib/common/strings.c``.  As
you can see, there are already tests for other functions in this same file in
the ``lib/common/tests/strings`` directory.

* cd into ``lib/common/tests/strings``
* Add the new file to the the ``check_PROGRAMS`` variable in ``Makefile.am``,
  making it something like this:

  .. code-block:: none

      check_PROGRAMS = \
             pcmk__add_word_test             \
             pcmk__btoa_test                 \
             pcmk__scan_port_test

* Create a new ``pcmk__scan_port_test.c`` file, copying the copyright and include
  boilerplate from another file in the same directory.
* Continue with the steps in `Writing the test`_.

Testing a function in a source file without tests
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Follow these steps if you want to test a function in a source file where there
are not already other tested functions, but there are tests for other files in
the same library.  For the purposes of this example, we will add a test for the
``pcmk_acl_required()`` function in ``lib/common/acls.c``.  At the time of this
documentation being written, no tests existed for that source file, so there
is no ``lib/common/tests/acls`` directory.

* Add to ``AC_CONFIG_FILES`` in the top-level ``configure.ac`` file so the build
  process knows to use directory we're about to create.  That variable would
  now look something like:

  .. code-block:: none

     dnl Other files we output
     AC_CONFIG_FILES(Makefile                                            \
                     ...
                     lib/common/tests/Makefile                           \
                     lib/common/tests/acls/Makefile                      \
                     lib/common/tests/agents/Makefile                    \
                     ...
     )

* cd into ``lib/common/tests``
* Add to the ``SUBDIRS`` variable in ``Makefile.am``, making it something like:

  .. code-block:: none

     SUBDIRS = agents acls cmdline flags operations strings utils xpath results

* Create a new ``acls`` directory, copying the ``Makefile.am`` from some other
  directory.  At this time, each ``Makefile.am`` is largely boilerplate with
  very little that needs to change from directory to directory.
* cd into ``acls``
* Get rid of any existing values for ``check_PROGRAMS`` and set it to
  ``pcmk_acl_required_test`` like so:

  .. code-block:: none

     check_PROGRAMS = pcmk_acl_required_test

* Double check that ``$(top_srcdir)/mk/tap.mk`` and ``$(top_srcdir)/mk/unittest.mk``
  are included in the ``Makefile.am``.  These files contain all the flags necessary
  for most unit tests.  If necessary, individual settings can be overridden like so:

  .. code-block:: none

     AM_CPPFLAGS += -I$(top_srcdir)
     LDADD += $(top_builddir)/lib/pengine/libpe_status_test.la

* Follow the steps in `Testing a new function in an already testable source file`_
  to create the new ``pcmk_acl_required_test.c`` file.

Testing a function in a library without tests
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Adding a test case for a function in a library that doesn't have any test cases
to begin with is only slightly more complicated.  In general, the steps are the
same as for the previous section, except with an additional layer of directory
creation.

For the purposes of this example, we will add a test case for the
``lrmd_send_resource_alert()`` function in ``lib/lrmd/lrmd_alerts.c``.  Note that this
may not be a very good function or even library to write actual unit tests for.

* Add to ``AC_CONFIG_FILES`` in the top-level ``configure.ac`` file so the build
  process knows to use directory we're about to create.  That variable would
  now look something like:

  .. code-block:: none

     dnl Other files we output
     AC_CONFIG_FILES(Makefile                                            \
                     ...
                     lib/lrmd/Makefile                                   \
                     lib/lrmd/tests/Makefile                             \
                     lib/services/Makefile                               \
                     ...
     )

* cd into ``lib/lrmd``
* Create a ``SUBDIRS`` variable in ``Makefile.am`` if it doesn't already exist.
  Most libraries should not have this variable already.

  .. code-block:: none

     SUBDIRS = tests

* Create a new ``tests`` directory and add a ``Makefile.am`` with the following
  contents:

  .. code-block:: none

     SUBDIRS = lrmd_alerts

* Follow the steps in `Testing a function in a source file without tests`_ to create
  the rest of the new directory structure.

* Follow the steps in `Testing a new function in an already testable source file`_
  to create the new ``lrmd_send_resource_alert_test.c`` file.

Adding to an existing test case
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

If all you need to do is add additional test cases to an existing file, none of
the above work is necessary.  All you need to do is find the test source file
with the name matching your function and add to it and then follow the
instructions in `Writing the test`_.

Writing the test
________________

A test case file contains a fair amount of boilerplate.  For this reason, it's
usually easiest to just copy an existing file and adapt it to your needs.  However,
here's the basic structure:

.. code-block:: c

   /*
    * Copyright 2021 the Pacemaker project contributors
    *
    * The version control history for this file may have further details.
    *
    * This source code is licensed under the GNU Lesser General Public License
    * version 2.1 or later (LGPLv2.1+) WITHOUT ANY WARRANTY.
    */

   #include <crm_internal.h>

   #include <crm/common/unittest_internal.h>

   /* Put your test-specific includes here */

   /* Put your test functions here */

   PCMK__UNIT_TEST(NULL, NULL,
                   /* Register your test functions here */)

Each test-specific function should test one aspect of the library function,
though it can include many assertions if there are many ways of testing that
one aspect.  For instance, there might be multiple ways of testing regular
expression matching:

.. code-block:: c

   static void
   regex(void **state) {
       const char *s1 = "abcd";
       const char *s2 = "ABCD";

       assert_true(pcmk__strcmp(NULL, "a..d", pcmk__str_regex) < 0);
       assert_true(pcmk__strcmp(s1, NULL, pcmk__str_regex) > 0);
       assert_int_equal(pcmk__strcmp(s1, "a..d", pcmk__str_regex), 0);
   }

Each test-specific function must also be registered or it will not be called.
This is done with ``cmocka_unit_test()`` in the ``PCMK__UNIT_TEST`` macro:

.. code-block:: c

   PCMK__UNIT_TEST(NULL, NULL,
                   cmocka_unit_test(regex))

Most unit tests do not require a setup and teardown function to be executed
around the entire group of tests.  On occassion, this may be necessary.  Simply
pass those functions in as the first two parameters to ``PCMK__UNIT_TEST``
instead of using NULL.

Assertions
__________

In addition to the `assertions provided by <https://api.cmocka.org/group__cmocka__asserts.html>`_,
``unittest_internal.h`` also provides ``pcmk__assert_asserts``.  This macro takes an
expression and verifies that the expression aborts due to a failed call to
``CRM_ASSERT`` or some other similar function.  It can be used like so:

.. code-block:: c

   static void
   null_input_variables(void **state)
   {
       long long start, end;

       pcmk__assert_asserts(pcmk__parse_ll_range("1234", NULL, &end));
       pcmk__assert_asserts(pcmk__parse_ll_range("1234", &start, NULL));
   }

Here, ``pcmk__parse_ll_range`` expects non-NULL for its second and third
arguments.  If one of those arguments is NULL, ``CRM_ASSERT`` will fail and
the program will abort.  ``pcmk__assert_asserts`` checks that the code would
abort and the test passes.  If the code does not abort, the test fails.


Running
_______

If you had to create any new files or directories, you will first need to run
``./configure`` from the top level of the source directory.  This will regenerate
the Makefiles throughout the tree.  If you skip this step, your changes will be
skipped and you'll be left wondering why the output doesn't match what you
expected.

To run the tests, simply run ``make check`` after previously building the source
with ``make``.  The test cases in each directory will be built and then run.
This should not take long.  If all the tests succeed, you will be back at the
prompt.  Scrolling back through the history, you should see lines like the
following:

.. code-block:: none

    PASS: pcmk__strcmp_test 1 - same_pointer
    PASS: pcmk__strcmp_test 2 - one_is_null
    PASS: pcmk__strcmp_test 3 - case_matters
    PASS: pcmk__strcmp_test 4 - case_insensitive
    PASS: pcmk__strcmp_test 5 - regex
    ============================================================================
    Testsuite summary for pacemaker 2.1.0
    ============================================================================
    # TOTAL: 33
    # PASS:  33
    # SKIP:  0
    # XFAIL: 0
    # FAIL:  0
    # XPASS: 0
    # ERROR: 0
    ============================================================================
    make[7]: Leaving directory '/home/clumens/src/pacemaker/lib/common/tests/strings'

The testing process will quit on the first failed test, and you will see lines
like these:

.. code-block:: none

   PASS: pcmk__scan_double_test 3 - trailing_chars
   FAIL: pcmk__scan_double_test 4 - typical_case
   PASS: pcmk__scan_double_test 5 - double_overflow
   PASS: pcmk__scan_double_test 6 - double_underflow
   ERROR: pcmk__scan_double_test - exited with status 1
   PASS: pcmk__starts_with_test 1 - bad_input
   ============================================================================
   Testsuite summary for pacemaker 2.1.0
   ============================================================================
   # TOTAL: 56
   # PASS:  54
   # SKIP:  0
   # XFAIL: 0
   # FAIL:  1
   # XPASS: 0
   # ERROR: 1
   ============================================================================
   See lib/common/tests/strings/test-suite.log
   Please report to users@clusterlabs.org
   ============================================================================
   make[7]: *** [Makefile:1218: test-suite.log] Error 1
   make[7]: Leaving directory '/home/clumens/src/pacemaker/lib/common/tests/strings'

The failure is in ``lib/common/tests/strings/test-suite.log``:

.. code-block:: none

   ERROR: pcmk__scan_double_test
   =============================

   1..6
   ok 1 - empty_input_string
   PASS: pcmk__scan_double_test 1 - empty_input_string
   ok 2 - bad_input_string
   PASS: pcmk__scan_double_test 2 - bad_input_string
   ok 3 - trailing_chars
   PASS: pcmk__scan_double_test 3 - trailing_chars
   not ok 4 - typical_case
   FAIL: pcmk__scan_double_test 4 - typical_case
   # 0.000000 != 3.000000
   # pcmk__scan_double_test.c:80: error: Failure!
   ok 5 - double_overflow
   PASS: pcmk__scan_double_test 5 - double_overflow
   ok 6 - double_underflow
   PASS: pcmk__scan_double_test 6 - double_underflow
   # not ok - tests
   ERROR: pcmk__scan_double_test - exited with status 1

At this point, you need to determine whether your test case is incorrect or
whether the code being tested is incorrect.  Fix whichever is wrong and continue.


Code Coverage
#############

Figuring out what needs unit tests written is the purpose of a code coverage tool.
The Pacemaker build process uses ``lcov`` and special make targets to generate
an HTML coverage report that can be inspected with any web browser.

To start, you'll need to install the ``lcov`` package which is included in most
distributions.  Next, reconfigure the source tree:

.. code-block:: none

   $ ./configure --with-coverage

Then run ``make -C devel coverage``.  This will do the same thing as ``make check``,
but will generate a bunch of intermediate files as part of the compiler's output.
Essentially, the coverage tools run all the unit tests and make a note if a given
line if code is executed as a part of some test program.  This will include not
just things run as part of the tests but anything in the setup and teardown
functions as well.

Afterwards, the HTML report will be in ``coverage/index.html``.  You can drill down
into individual source files to see exactly which lines are covered and which are
not, which makes it easy to target new unit tests.  Note that sometimes, it is
impossible to achieve 100% coverage for a source file.  For instance, how do you
test a function with a return type of void that simply returns on some condition?

Note that Pacemaker's overall code coverage numbers are very low at the moment.
One reason for this is the large amount of code in the ``daemons`` directory that
will be very difficult to write unit tests for.  For now, it is best to focus
efforts on increasing the coverage on individual libraries.

Additionally, there is a ``coverage-cts`` target that does the same thing but
instead of testing ``make check``, it tests ``cts/cts-cli``.  The idea behind this
target is to see what parts of our command line tools are covered by our regression
tests.  It is probably best to clean and rebuild the source tree when switching
between these various targets.


Debugging
#########

gdb
___

If you use ``gdb`` for debugging, some helper functions are defined in
``devel/gdbhelpers``, which can be given to ``gdb`` using the ``-x`` option.

From within the debugger, you can then invoke the ``pcmk`` command that
will describe the helper functions available.