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

Change Assert to AssertThrow in the tests. #688

Merged
merged 1 commit into from Mar 28, 2015

Conversation

Rombur
Copy link
Member

@Rombur Rombur commented Mar 24, 2015

This fixes #598 It's not perfect but it's much better than what we had. I've run the testsuite and it's seems to work (http://cdash.kyomu.43-1.org/buildSummary.php?buildid=5031the warnings are due to the version of openmpi, I'm using).

@bangerth
Copy link
Member

Before we do this, I'd like to share an observation I mad the other day. If you run into a testcase that uses AssertThrow or throws an exception directly, then right now you simply end up with a testcase that silently aborts. It will return a nonzero error code, but the exception is not printed on the screen. So, if we change Assert to AssertThrow everywhere, we will have to learn to deal with testcases that look like the run through just fine when you call them by hand on the command line, but that do not in fact. They're also much more awkward to debug since the debugger no longer just stops at the site of the error :-(

@Rombur
Copy link
Member Author

Rombur commented Mar 25, 2015

I can go back and add try and catch in all the tests. If I can catch all the exceptions at once it will be easy. I could also just create a new Assert that also works in release mode.

@tamiko
Copy link
Member

tamiko commented Mar 25, 2015

Please give me half a day to think about this. :-)

@bangerth: This is a bit surprising. A test should set abort_on_exception to false which implies that all Assert are also thrown (exception.cc:336). Or what am I missing?

@bangerth
Copy link
Member

@tamiko : Only a few tests actually set abort_on_exception to false. The vast majority do actually abort the program and print the error message.

@bangerth
Copy link
Member

We could also try to investigate why this piece in exceptions.cc doesn't seem to work:

// Newer versions of gcc have a very nice feature: you can set a verbose
// terminate handler, that not only aborts a program when an exception is
// thrown and not caught somewhere, but before aborting it prints that an
// exception has been thrown, and possibly what the std::exception::what()
// function has to say. Since many people run into the trap of not having a
// catch clause in main(), they wonder where that abort may be coming from.
// The terminate handler then at least says what is missing in their
// program.
#ifdef DEAL_II_HAVE_VERBOSE_TERMINATE
namespace __gnu_cxx
{
  extern void __verbose_terminate_handler ();
}

namespace
{
  struct preload_terminate_dummy
  {
    preload_terminate_dummy()
    {
      std::set_terminate(__gnu_cxx::__verbose_terminate_handler);
    }
  };

  static preload_terminate_dummy dummy;
}
#endif

@tjhei
Copy link
Member

tjhei commented Mar 25, 2015

right now you simply end up with a testcase that silently aborts

This is because we are not writing cout/cerr into the test output file, right? If I run a test with ctest -V I can see exception information on the screen with Assert and AssertThrow.

@bangerth
Copy link
Member

Uh, maybe I misremember? I had a case where an exception was thrown via throw "error text" and nothing was shown on the screen. If you have AssertThrow instead of throw, you do get text on the screen? In that case, I'm retracting my objections.

@tjhei
Copy link
Member

tjhei commented Mar 25, 2015

If I do throw "bla" I get

    terminate called after throwing an instance of 'char const*'
    Aborted (core dumped)

on screen. If I use Assert or AssertThrow I get a nice exception msg. Of course neither of these is part of the test output.

bangerth added a commit that referenced this pull request Mar 28, 2015
Change Assert to AssertThrow in the tests.
@bangerth bangerth merged commit e1ccb16 into dealii:master Mar 28, 2015
@tamiko
Copy link
Member

tamiko commented Mar 28, 2015

This is because we are not writing cout/cerr into the test output file, right?

Exactly - we're not doing this at the moment.

@Rombur Rombur deleted the AssertThrow branch April 27, 2015 14:14
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.

Assert vs AssertThrow in the testsuite
4 participants