Skip to content

Contribute

Jump to bottom
craffael edited this page Jun 24, 2018 · 18 revisions

You are encouraged to contribute fixes and improvements to the codebase of LehrFEM++. However there are a few things to consider before making a pull request:

  1. Follow our coding standard
  2. Include unit/integration tests if possible

1. Style guide

1.1. Naming conventions from Google C++ Style Guide

  • Filenames should be all lowercase and can include underscores (_) or dashes (-).
  • Type names start with a capital letter and have a capital letter for each new word, with no underscores. For clarity the suffix _t should be appended1.
  • The names of variables (including function parameters) and data members are all lowercase, with underscores between words.
  • Data members of classes (not structs), both static and non-static, are named like ordinary non-member variables, but with a trailing underscore.
  • Regular functions have mixed case; accessors and mutators may be named like variables. Ordinarily, functions should start with a capital letter and have a capital letter for each new word.
  • Namespace names are all lower-case.
  • Macros are all uppercase.
  • Variables declared constexpr or const, and whose value is fixed for the duration of the program, are named with a leading "k" followed by mixed case.

1.2. Conventions for commenting

  • Throughout use /** */ and JavaDoc style for Doxygen documentation, never use // or /* */ for this purpose.
  • Use // comments only for explaining details of variables and algorithms inside functions. You may use LaTeX typesetting elements in // comments.

1.3. Variable names

  • short variable names may be used, but then the variable has to be explained by a comment at the site of declaration.

1.4. clang-format and clang-tidy

LehrFEM++ relies on clang-format and clang-tidy to make sure the codebase conforms to the styleguide described above. Whenver you make a pull request or push a commit to the repository, these two tools will run on travis-ci.org and you will get error messages (by email) if they fail. Feature branches and Pull Requests are not merged into master before they build successfully!

clang-format

When clang-format fails on travis, the build output on travis-ci will show you how you should format your files. In principle you can then make the appropriate changes and push another commit. However, it's much easier if you let clang-format make these changes for you:

  • Make sure you have a recent version of clang-format on your computer (e.g. clang-format-5.0 or clang-format-6.0.
  • To reformat a single file run clang-format -i <path to file>
  • To check if all your files have been formatted correctly, run the bash script run_clang_format.sh in the root folder of this repository (on linux). This will apply the style that is specified in the file .clang-format.

There exist integrations for many popular editors that apply clang-format for instance whenever you save a file. This is a huge help since you don't have to think about formatting at all anymore :)

clang-tidy

clang-tidy is a lint-tool that points out common programming mistakes. When it fails on travis-ci you will also see a list of errors that must be corrected. You can either correct these mistakes based on the output at travis-ci or run clang-tidy locally on your machine before every commit (recommended):

  • Make sure you have a recent version of clang-tidy on your computer (e.g. clang-tidy-5.0 or clang-tidy-6.0)
  • change into a CMake build directory and make sure that the CMake variable CMAKE_EXPORT_COMPILE_COMMANDS is set: cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=On .. This will create the file compile_commands.json.
  • To run clang-tidy over the whole repository, execute the bash script run_clang_tidy.sh from the cmake build folder (linux only).
  • Correct the errors that clang-tidy complains about. Note that clang-tidy can fix some problems automatically, see the -fix command line option.

2. Testing

LehrFEM++ makes extensive use of the Google's C++ test framework. Adding a test is very simple and we encourage you to submit/change tests whenever you submit a pull request:

  • If you add new functionality, write new unit/integration test(s) that test this functionality.
  • If you fix a bug, write a unit/integration test that tests this bug.

2.1. How to add a test

Lets say you want to add a new test for the class lf::geometry::QuadO1. Then you have to do two things:

  1. Check if the file lib/lf/geometry/test/quad_o1_tests.cc already exists. If not, create it and register it in lib/lf/geometry/test/CMakeLists.txt.
  2. Write your tests into the quad_o1_tests.cc file. Since you will be testing the class lf::geometry::QuadO1 you should put your tests into a Google Test Case/Suite with the name QuadO1. The tests should be defined inside the namespace lf::geometry::test. I.e. your test file should look similar to this:
#include <gtest/gtest.h>
// more includes
namespace lf::geometry::test {
TEST(QuadO1, Jacobian) {
  // test for QuadO1::Jacobian() here...
}
}

Implicit in the above description are the following conventions:

  • There is one C++ source test file for each class that should be tested. If the class x::y::Z lies in module/namespace x::y then the C++ source file that contains the tests should be placed at lib/x/y/test/z_tests.cc.
  • The source file should use the lower_underscore naming convention and end with _tests.cc.
  • There is one Google Test Case/Suite for each class that is to be tested. For the class x::y::Z, the test case should be named Z and all tests should be defined in the namespace x::y::test.

See the Google Test Primer for more information about writing tests.

2.2. Reusing tests

Since LehrFEM++ relies heavily on abstract interface classes, it is often possible to write test routines that can be used to test all/most classes that implement a given interface. Lets say we want to write a test routine that checks the output of the lf::geometry::Geometry::Jacobian() method by numeric differentiation. Then the procedure is as follows:

  1. Create the file lib/lf/geometry/test/check_jacobian.h and declare a function that accepts any Geometry object and tests the jacobians:
#ifndef <__include_guard_here>
#define <__include_guard_here>

#include <gtest/gtest.h>
#include <lf/geometry/geometry.h>

namespace lf::geometry::test {
void checkJacobian(const Geometry& geometry);
}
#endif
  1. Because checkJacobian() is a normal function (and not a function template), we can define it in a separate C++ source file (which saves compilation time). We place it at lib/lf/geometry/test/check_jacobian.cc:
#include "check_jacobian.h"

namespace lf::geometry::test {
void checkJacobian(const Geometry& geometry) {
  // Check implementation here with automatic differentiation.
}
}
  1. Register check_jacobian.h and check_jacobian.cc in lib/lf/geometry/test/CMakeLists.txt
  2. Create a corresponding test for each class where we want to test the jacobian. E.g. we would could add the following test to lib/lf/geometry/test/quad_o1.cc:
#include "check_jacobian.h"
namespace lf::geometry::test {
TEST(QuadO1, Jacobian) {
  QuadO1 geom = ...;
  checkJacobian(geom);
}
}

2.2.1. Test Modules

Sometimes it makes sense to make check routines available to other modules/executables. Such routines should be put into test_utils modules. These test_utils modules can then be imported by other test modules/executables.

An example of this is the function lf::mesh::test_utils::testEntityIndexing(const Mesh& mesh) which takes a mesh and checks that the indices of the entities are all consecutive. Since this method works with any Mesh object, it doesn't make sense to put it e.g. into lf::mesh::hybrid2d::test since the tests in lf::mesh::hybrid3d::test (doesn't exist yet) probably also want to use it.

The lf::mesh::hybrid2d::test executable can then import the lf::mesh::test_utils module and call the method testEntityIndexing.

1: Strictly speaking, this is an extension of the Google Coding style guide.

Clone this wiki locally