Add OSX builds and refactor TravisCI build config #317
Conversation
Replaces the apt-get command with the apt addon.
Make the build matrix more explicit. Only check if it's a cron job when setting the variables for build actions. This way, we can overwrite that to run full builds by setting those variables to "true". Rename the build scripts to more descriptive names.
Avoid downloading and installing miniconda.
Testing is known to be failing so use a different job to build and deploy docs.
leouieda
changed the title
leouieda
changed the title
leouieda
changed the title
|
I think I managed to get a simpler setup than #146 and the build time for OSX is 7.5 min. Even then, we always have the option of ignoring the CI (though we shouldn't). Running a last test to see if the build works on the cron jobs as well. |
.travis.yml
Outdated
| - name: "Mac" | ||
| os: osx | ||
| if: type != cron | ||
| - name: "Max" |
| set (DCW_ROOT "$ENV{COASTLINEDIR}") | ||
| set (GSHHG_ROOT "$ENV{COASTLINEDIR}") | ||
|
|
||
| set (CMAKE_BUILD_TYPE Debug) |
There was a problem hiding this comment.
Duplicate CMAKE_BUILD_TYPE if "COVERAGE" == "true".
There was a problem hiding this comment.
👍 removed the one inside the if block
.travis.yml
Outdated
| - cd .. | ||
|
|
||
| # Things to do if the build is successful | ||
| after_success: |
There was a problem hiding this comment.
Although some tests fail, we can still upload coverage reports.
There was a problem hiding this comment.
Right, I forgot about that. At least now the tests really show up as failures instead of passing. Hopefully that red badge will be good motivation to fix them or mark as won't fix 😉
leouieda
changed the title
|
@seisman should be good to go. Could you do one last review? |
.travis.yml
Outdated
| export CONDA_INSTALL_EXTRA="sphinx" | ||
| export COVERAGE=true | ||
| fi | ||
| - COVERAGE=true |
There was a problem hiding this comment.
The environment variable COVERAGE seems useless, since we always need to run the tests to generate coverage reports.
There was a problem hiding this comment.
Yeah, I agree. I'll remove it and use the TEST variable instead.
| cat >> cmake/ConfigUser.cmake << 'EOF' | ||
| set (CMAKE_C_FLAGS "-Wall -Wdeclaration-after-statement -coverage -O0") | ||
| EOF | ||
| fi |
There was a problem hiding this comment.
We don't need to enable the tests in compile-only jobs, so we can change it to:
if [ "$TEST" == "all"]; then
cat >> cmake/ConfigUser.cmake << 'EOF'
set (CMAKE_BUILD_TYPE Debug)
enable_testing()
set (DO_EXAMPLES TRUE)
set (DO_TESTS TRUE)
set (DO_ANIMATIONS TRUE)
set (N_TEST_JOBS 2)
set (CMAKE_C_FLAGS "-Wall -Wdeclaration-after-statement -coverage -O0")
EOF
fi
There was a problem hiding this comment.
Are any of those required for building the docs?
There was a problem hiding this comment.
Never mind, already found it.
|
@leouieda The PR looks good. I just made two comments. |
|
@leouieda codecov needs a token to upload the coverage reports. The token was added via the travis settings panel. You can encrypt this token and add it to .travis.yml. |
No need. Plus, I can't access the unencrypted token from the web interface. |
Refactor the TravisCI build configuration to make it easier to add OSX support
and to deploy the docs to Github pages later on. Set different builds in the
matrix for cron jobs and separate testing and building the docs. This is
necessary since the tests are known to always fail and we want to deploy the
docs anyway. The build was always passing even if the docs or tests ended in
failure because the
cd build; ...; cd..;command always passes. The returnvalue is only taken from the last
cd.