Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reusing test suites #310

Merged
merged 4 commits into from
May 17, 2019
Merged

Conversation

phil-blain
Copy link
Member

Small modifications to be able to re-run test suites with incremental compilation.

  • Add the incremental option (sets ICE_CLEANBUILD to false)
  • Rename the suite.run file to suite.submit and add a suite.run file that runs the suite interactively
  • Delete the ciceexe.* executables at the begining of suite.run and suite.submit so that the incremental option can be effective.
  • Developer(s): Philippe Blain

  • Please suggest code Pull Request reviewers in the column at right. @apcraig

  • Are the code changes bit for bit, different at roundoff level, or more substantial?
    BFB - no change to code.

  • Please include the link to test results or paste the summary block from the bottom of the testing output below.

  • Does this PR create or have dependencies on Icepack or any other models?
    No. If you agree with these changes I can make a similar PR for Icepack.

  • Is the documentation being updated with this PR? (Y/N) Yes
    Example 11 for test suites,
    Some hints,
    Preset options
    If not, does the documentation need to be updated separately at a later time? (Y/N)

Note: "Documentation" includes information on the wiki and .rst files in doc/source/,
which are used to create the online technical docs at https://readthedocs.org/projects/cice-consortium-cice/.

  • Other Relevant Details:
    I also did some minor editing of the documentation, including fixing some indentation.

Copy link
Contributor

@apcraig apcraig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay in looking at this. I think there may be some challenges with rerunning suites in general, like whether tests will rerun cleanly after either succeeding or failing, whether you want to rerun tests that already ran and passed, whether you just want to rerun on tests that were not run or on tests that failed or on all tests. It might be nice to be able to choose some of that when running the suite again "manually". But I think this is a good step forward.

My main comment is that I don't think "incremental" is a good name for the option. How about "nocleanbuild" or something like it. It could be that settings (-s) will be use for setting up cases and other things and "incremental" is not meaningful when all it's doing is resetting the CLEANBUILD default. If we can make that change, I think we can merge the PR. Thanks!

@phil-blain
Copy link
Member Author

Thanks for your comments.

some challenges with rerunning suites in general, like whether tests will rerun cleanly after either succeeding or failing

From what I tested, the tests run cleanly after success or failure.

whether you want to rerun tests that already ran and passed, whether you just want to rerun on tests that were not run or on tests that failed or on all tests

You are right that some additional flexibility could be good, maybe for a latter PR. For now, all tests are rerun.

Regarding the name of the option, would "buildincremental" work, or "incrementalbuild' ? I just thought that since it's pretty standard terminology in software engineering, we might as well use it. But It's not that important.

@apcraig
Copy link
Contributor

apcraig commented May 13, 2019

I see, you are using "incremental" to mean a typical make build based on the current build without a clean. I'm not sure I've heard that term used in that way before, but it could be. I would propose that we add two new settings, called either "cleanbuild" and "nocleanbuild" OR "buildclean" and "nobuildclean" OR "buildincremental" and "buildclean". I think the last (buildincremental and buildclean) is probably my preferred option at this point. So, if you agree, could you change the "incremental" to "buildincremental" and while you are doing that, add another option which is "buildclean" which is the current default, but I think it's worth adding it as a setting if we are adding the other. Thanks!

@apcraig apcraig merged commit 37b799c into CICE-Consortium:master May 17, 2019
@phil-blain phil-blain deleted the reusing-test-suites branch August 30, 2019 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants