Skip to content

Latest commit

 

History

History
283 lines (236 loc) · 9.98 KB

CONTRIBUTING.md

File metadata and controls

283 lines (236 loc) · 9.98 KB

Contributing

This document assumes you have followed the Apache Geode Code contribution instructions

Building the code

see BUILDING.md

Next steps

Testing

Before submitting a pull request all tests must pass. This includes all unit tests, integration tests, and acceptance tests. We are using CTest for running tests (please see the CTest documentation for further information) and Google Test as testing framework.

Running unit tests

$ cd build/cppcache/test/<Debug|Release|if needed>
$ ./apache-geode_unittests

Running integration tests

There are two test suites of integration tests based on different testing frameworks. The old integration tests (stored in geode-native/cppcache/integration-test) are based on a custom testing framework, and the newer ones (stored in geode-native/cppcache/integration/test) are based on Google Test.

Old integration tests are deprecated. If your changes include the implementation of new integration test/s to be verified, they should be written using Google Test. If your change implies a significant change in one or more old test cases, you should create the equivalent test case/s using Google Test to substitute the old one/s instead of adapting them.

Both integration test suites can be executed using CTest.

Running heritage integration test suite

$ cd build/cppcache/integration-test
$ ctest -C <Debug|RelWithDebInfo> --timeout 300 -j1

Execution will take 2 hours approximately. It is possible to increase the number of jobs changing the value of -j parameter (up to 4) for running tests in parallel, although it may end up with failed tests that will need to be re-run sequentially.

Standalone tests can also be run as follows:

$ cd build/cppcache/integration-test
$ ctest -C <Debug|RelWithDebInfo> -R <test_name>

For example:

$ ctest -C RelWithDebInfo -R testCacheless

.NET integration tests can be executed similarly from build/clicache/integration-test.

Running new Google Test integration test suite

Make sure Docker is installed as it is required for SNI Tests

$ cd build/cppcache/integration/test
$ ctest -C <Debug|RelWithDebInfo> -j1

It is possible to increase the number of jobs changing the value of -j parameter for running tests in parallel, although it may end up with failed tests that will need to be re-run sequentially. Standalone tests can also be run as follows:

$ cd build/cppcache/integration/test
$ ctest -C <Debug|RelWithDebInfo> -R <test_name> -j1

For example:

$ ctest -C RelWithDebInfo -R AuthInitializeTest.putGetWithBasicAuth -j1

Notice that BasicIPv6Test test is expected to fail due to IPv6 support is disabled by default. BUILDING.md explains how to enable it.

Running acceptance tests

Acceptance tests is a new category of tests that are designed to test end to end connectivity of a geode-native client with a cloud based geode cluster that sits behind a proxy server.

These tests are stored in geode-native/cppcache/acceptance-test and geode-native/clicache/acceptance-test for C++ and .NET clients respectively. They utilize docker containers for the proxy server and geode servers. They are enabled during cmake configuration only if docker and docker-compose are found.

The acceptance tests can be run using ctest as follows. Note: Currently, the tests can only be run sequentially, hence the -j1 flag below.

For C++ clients:

$ cd build/cppcache/acceptance-test
$ ctest -C <Debug|RelWithDebInfo> -R <test_name> -j1

For .NET clients:

$ cd build/clicache/acceptance-test
$ ctest -C <Debug|RelWithDebInfo> -R <test_name> -j1

Style

Formatting C++

For C++ it is required to follow the Google C++ Style Guide and have a build target that uses clang-format to achieve compliance.

Clang-format

Individual targets in the build tree have their own dependency of the form <<targetName>>-clangformat, which uses the clang-format executable, wherever it is found, to format and modified files according to the rules specfied in the .clang-format file. This is helpful when submitting changes to geode-native, because an improperly formatted file will fail Travis-CI and have to be fixed prior to merging any pull request. If clang-format is not installed on your system, clangformat targets will not be added to your project files, and geode-native should build normally. Under some circumstances, however, it may become necessary to disable clang-format on a system where it is installed. The clang-format rules are defined in the .clang-format files.

To enable clang-tidy if CMake does not find clang-format:

$ cmake … -DClangFormat_EXECUTABLE=/path/to/clang-format …

To disable clang-format in the build:

$ cmake … -DClangFormat_EXECUTABLE=''

On the other hand, it may also be desirable to run clang-format on the entire source tree. This is also easily done via the all-clangformat in a build with clang-format enabled. If clang-format has been disabled in the cmake configuration step, as above, the all-clangformat target will not exist, and the cmake configuration step will have to be re-run with clang-format enabled.

To run clang-format on the entire source tree:

$ cmake --build . --target all-clangformat

Clang-Tidy

Code style and conventions are enforced with clang-tidy. The clang-tidy rules can be found in the .clang-tidy files.

To enable clang-tidy:

$ cmake … -DCMAKE_CXX_CLANG_TIDY=/path/to/clang-tidy …

Code Cleanup

When editing old code please make the following changes.

  • Prefer the use of auto C++ or var in C# where applicable.

    std::vector<Cars> cars = lot.getCars();

    should be changed to

    auto cars = lot.getCars();
  • Prefer range for loops over traditional for loops where applicable.

    for (std::vector<Car>::iterator i = cars.begin(); i != cars.end(), ++i) {
      Car car = *i;
      std::cout << car.getName();
    }

    should be changed to

    for (const auto& car : cars) {
      std::cout << car.getName();
    }
  • Fix bad variable names. Variable names should be expressive.

    double i = car.getFuelLevel();

    should be changed to

    auto fuelLevel = car.getFuelLevel();
  • Use override on all method overrides.

    Given a base class

    class Base {
     public:
      Base();
      virtual ~Base();
      virtual virtualMethod();
      virtual pureVirtualMethod() = 0;
    }

    the derived class

    class Derived : public Base {
     public:
      virtual ~Derived();
      virtual virtualMethod();
      virtual pureVirtualMethod();
    }

    should be changed to

    class Derived : public Base {
     public:
      Derived();
      ~Derived() override;
      virtualMethod() override;
      pureVirtualMethod() override;
    }
  • Fix std::string::c_str() calls where passed to std::string parameters.

    auto name = std::string("Delorean");
    ...
    car.setName(name.c_str());

    should be changed to

    auto name = std::string("Delorean");
    ...
    car.setName(name);
  • Replace sprintf for logging messages with std::string or std::stringstream.

    char[1024] buffer;
    sprintf(buffer, "Car::crashed: name=%s", car.getName().c_str());
    LOG(buffer);

    should be changed to

    LOG("Car::crashed: name=" + car.getName());
  • Replace dynamic_cast on std::shared_ptr with std::dynamic_pointer_cast. Same goes for static_cast to std::static_pointer_cast, etc.

    std::shared_ptr<Car> car = garage.getCar();
    Delorean* delorean = dynamic_cast<Delorean*>(car.get());
    if (nullptr != delorean) {
      delorean->setSpeed(88);
    }

    should be changed to

    auto car = garage.getCar();
    if (auto delorean = std::dynamic_pointer_cast<Delorean>(car)) {
      delorean->setSpeed(88);
    }

Code Coverage

If building with GCC or Clang you can enable C++ code coverage by adding -DUSE_CPP_COVERAGE=ON to the CMake generator command.

$ cmake .. -DUSE_CPP_COVERAGE=ON …

You can then generate a C++ code coverage report by downloading lcov. After acquiring lcov, finish the Steps to build section above. Then, run the tests as described in the this document. Finally, run the following commands from the build directory:

$ lcov --capture --directory . --output-file coverage.info
$ genhtml coverage.info --output-directory coverage_report

You can then open the index.html file in the coverage_report directory using any browser.