From bc02ada4b008a8541c3788a18a8d8eca401f9ac4 Mon Sep 17 00:00:00 2001 From: Lotte Steenbrink Date: Fri, 4 Jan 2019 15:39:22 +0100 Subject: [PATCH 1/2] Add build instructions to contributing.md --- docs/contributing.md | 49 +++++++++++++++++++++++++++++++------------- 1 file changed, 35 insertions(+), 14 deletions(-) diff --git a/docs/contributing.md b/docs/contributing.md index b6150375..d7fb4c53 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,41 @@ 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 -* 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 -Bcmake-build-debug -H. -DCATCH_BUILD_TESTING=ON +$ cd cmake-build-debug/ +$ make +``` +-- 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: +``` +$ ./cmake-build-debug/projects/SelfTest +``` +There should be no unexpected failures on simple run. +*Note: on Windows, the binary is located at `cmake-build-debug\projects\Debug\SelfTest`* + +* Running Catch's approval tests: +``` +$ ./scripts/approvalTests.py +``` +Approval tests compare current output of the SelfTest binary in various +configurations against known good output. If you've configured the SelfTest +binary with your changes to be located in any other directory than +`/cmake-build-debug/projects/SelfTest`, its path needs to be passed to the +script, like so: `$ ./scripts/approvalTests.py clang-build/SelfTest`. +The output should be fairly self-explanatory. From 2988e9f6cf1365706d6a937581b85b331666252c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ho=C5=99e=C5=88ovsk=C3=BD?= Date: Sat, 19 Jan 2019 13:04:45 +0100 Subject: [PATCH 2/2] Update contributing.md to reflect current test setup --- docs/contributing.md | 43 ++++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/docs/contributing.md b/docs/contributing.md index d7fb4c53..21bbad14 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -42,36 +42,37 @@ 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: ``` $ cd Catch2 -$ cmake -Bcmake-build-debug -H. -DCATCH_BUILD_TESTING=ON -$ cd cmake-build-debug/ -$ make +$ cmake -Bdebug-build -H. -DCMAKE_BUILD_TYPE=Debug +$ cmake --build debug-build ``` --- 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. +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 SelfTest binary: -``` -$ ./cmake-build-debug/projects/SelfTest -``` -There should be no unexpected failures on simple run. -*Note: on Windows, the binary is located at `cmake-build-debug\projects\Debug\SelfTest`* -* Running Catch's approval tests: +* Running the tests via CTest: ``` -$ ./scripts/approvalTests.py +$ cd debug-build +$ ctest -j 2 --output-on-failure ``` -Approval tests compare current output of the SelfTest binary in various -configurations against known good output. If you've configured the SelfTest -binary with your changes to be located in any other directory than -`/cmake-build-debug/projects/SelfTest`, its path needs to be passed to the -script, like so: `$ ./scripts/approvalTests.py clang-build/SelfTest`. -The output should be fairly self-explanatory. +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.