search

edoddridge / aronnax

An idealised isopycnal model that can be run either with n+1/2 layers, or with n layers and variable bathymetry.
http://aronnax.readthedocs.io/en/latest/
MIT License
24 stars 6 forks source link

Build exit status #20

Closed axch closed 7 years ago

axch commented 7 years ago

Pursuant to Issue #7, change the run_examples.sh script to emit a failing exit status if any of the compilations or model runs fail.

It currently indeed fails, but I am not sure how to fix it.

axch commented 7 years ago

Ironically, fixing run_examples.sh causes this PR to fail Travis checking. However, it does fix #7.

edoddridge commented 7 years ago

It looks like this is failing the test because the fu.bin input for the beta_plane_gyre test is too small.

I think this could be due to the examples trying to use the input files from before the regrid branch was merged in. The test cases that successfully ran all use square domains, so the total number of grid points was not affected by the changes in the regrid branch. I think perhaps I didn't replace the input files when I merged.

edoddridge commented 7 years ago

Now that we've confirmed Travis CI will fail if the tests fail, do you want to close #7?

edoddridge commented 7 years ago

Well, that's not what I expected. After regenerating the inputs on my laptop the examples all ran perfectly.

axch commented 7 years ago

Re: closing #7: I think that the fix #7 annotation in my comment will cause Github to close that ticket when this PR is merged. Or perhaps an annotation like that needs to appear in the log message of the merge commit, which you may or may not have the option to type in when you merge the PR. In any case, I strongly prefer that the open/closed status of issues reflect the master branch, so would rather wait to close #7 until this is merged.

axch commented 7 years ago

Re: commit discipline: Dunno. I've never seen anyone do recursive pull requests. Generally, what people tend to do is commit on top of the PR branch if the PR is "broken" (according to the project's policy), or else merge and then make a separate change if the PR is "acceptable" on its own terms but brings up another issue that needs to be resolved.

Note that in an instance like this, it doesn't make all that much sense to stick with a strict "Travis must always pass on master" policy, because the reason it was passing before this PR is because the build script incorrectly reported a pass in a broken state.

axch commented 7 years ago

Speaking of Travis, it seems that it and your laptop disagree about whether the examples run successfully: https://travis-ci.org/edoddridge/MIM/builds/205668869