diff --git a/docs/contributing.md b/docs/contributing.md index b6150375..21bbad14 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -1,8 +1,8 @@ # Contributing to Catch -So you want to contribute something to Catch? That's great! Whether it's a bug fix, a new feature, support for -additional compilers - or just a fix to the documentation - all contributions are very welcome and very much appreciated. +So you want to contribute something to Catch? That's great! Whether it's a bug fix, a new feature, support for +additional compilers - or just a fix to the documentation - all contributions are very welcome and very much appreciated. Of course so are bug reports and other comments and questions. If you are contributing to the code base there are a few simple guidelines to keep in mind. This also includes notes to @@ -16,7 +16,7 @@ Ongoing development is currently on _master_. At some point an integration branc ## Directory structure -_Users_ of Catch primarily use the single header version. _Maintainers_ should work with the full source (which is still, +_Users_ of Catch primarily use the single header version. _Maintainers_ should work with the full source (which is still, primarily, in headers). This can be found in the `include` folder. There are a set of test files, currently under `projects/SelfTest`. The test app can be built via CMake from the `CMakeLists.txt` file in the root, or you can generate project files for Visual Studio, XCode, and others (instructions in the `projects` folder). If you have access to CLion, @@ -37,20 +37,42 @@ as these are managed by the scripts!__ ## Testing your changes -Obviously all changes to Catch's code should be tested. If you added new functionality, you should add tests covering and -showcasing it. Even if you have only made changes to Catch internals (ie you implemented some performance improvements), -you should still test your changes. +Obviously all changes to Catch's code should be tested. If you added new +functionality, you should add tests covering and showcasing it. Even if you have +only made changes to Catch internals (i.e. you implemented some performance +improvements), you should still test your changes. -This means 3 things +This means 2 things -* Compiling Catch's SelfTest project -- code that does not compile is evidently incorrect. Obviously, you are not expected to -have access to all compilers and platforms Catch supports, Catch's CI pipeline will compile your code using supported compilers -once you open a PR. -* Running the SelfTest binary. There should be no unexpected failures on simple run. -* Running Catch's approval tests. Approval tests compare current output of the SelfTest binary in various configurations against -known good output. Catch's repository provides utility scripts `approvalTests.py` to help you with this. It needs to be passed -the SelfTest binary compiled with your changes, like so: `$ ./scripts/approvalTests.py clang-build/SelfTest`. The output should -be fairly self-explanatory. +* Compiling Catch's SelfTest project: +``` +$ cd Catch2 +$ cmake -Bdebug-build -H. -DCMAKE_BUILD_TYPE=Debug +$ cmake --build debug-build +``` +because code that does not compile is evidently incorrect. Obviously, +you are not expected to have access to all the compilers and platforms +supported by Catch2, but you should at least smoke test your changes +on your platform. Our CI pipeline will check your PR against most of +the supported platforms, but it takes an hour to finish -- compiling +locally takes just a few minutes. + + +* Running the tests via CTest: +``` +$ cd debug-build +$ ctest -j 2 --output-on-failure +``` +If you added new tests, approval tests are very likely to fail. If they +do not, it means that your changes weren't run as part of them. This +_might_ be intentional, but usually is not. + +The approval tests compare current output of the SelfTest binary in various +configurations against known good outputs. The reason it fails is, +_usually_, that you've added new tests but have not yet approved the changes +they introduce. This is done with the `scripts/approve.py` script, but +before you do so, you need to check that the introduced changes are indeed +intentional.