REQUIRE_THROWS_AS doesn't work with constructor

[ruipacheco]

I've upgraded to v.1.7.0 and tests that used to run are now breaking.

Steps to reproduce

This test used to run and now doesn't:

TEST_CASE("Test Server settings.") {
  Parameters params;
  params.engine = EngineType::Server;

  SECTION("Test constructor exceptions.") {
    SECTION("Throw exception if username has more than 63 characters.") {
      params.username = "E0DFC0B4863012DA7F3BF2FBD57601C70D99A39398CDF6D412815FE92B336953";  // 64 chars
      REQUIRE_THROWS_AS(AGConnection conn(params), std::logic_error);
    }

    SECTION("Throw exception if server name has more than 63 characters.") {
      params.db_name = "E0DFC0B4863012DA7F3BF2FBD57601C70D99A39398CDF6D412815FE92B336953";  // 64 chars
      REQUIRE_THROWS_AS(AGConnection conn(params), std::logic_error);
    }
  }
}

And I get the error:

/Users/ruihpacheco/chi/databaseclient/cpp/tests/unit/params.cpp:17:25: error: 'AGConnection' does not refer to a value
      REQUIRE_THROWS_AS(AGConnection conn(params), std::logic_error);
                        ^
../tests/../src/internal/postgresql/AGConnection.h:28:13: note: declared here
      class AGConnection : public AbstractConnection<AGConnection> {
            ^
/Users/ruihpacheco/chi/databaseclient/cpp/tests/unit/params.cpp:17:38: error: expected ')'
      REQUIRE_THROWS_AS(AGConnection conn(params), std::logic_error);
                                     ^
/Users/ruihpacheco/chi/databaseclient/cpp/tests/unit/params.cpp:17:7: note: to match this '('
      REQUIRE_THROWS_AS(AGConnection conn(params), std::logic_error);
      ^
catch-src/single_include/catch.hpp:10935:50: note: expanded from macro 'REQUIRE_THROWS_AS'
#define REQUIRE_THROWS_AS( expr, exceptionType ) INTERNAL_CATCH_THROWS_AS( expr, exceptionType, Catch::ResultDisposition::Normal, "REQUIRE_THROWS_AS" )
                                                 ^
catch-src/single_include/catch.hpp:2283:34: note: expanded from macro 'INTERNAL_CATCH_THROWS_AS'
                static_cast<void>(expr); \
                                 ^
/Users/ruihpacheco/chi/databaseclient/cpp/tests/unit/params.cpp:22:25: error: 'AGConnection' does not refer to a value
      REQUIRE_THROWS_AS(AGConnection conn(params), std::logic_error);
                        ^
../tests/../src/internal/postgresql/AGConnection.h:28:13: note: declared here
      class AGConnection : public AbstractConnection<AGConnection> {
            ^
/Users/ruihpacheco/chi/databaseclient/cpp/tests/unit/params.cpp:22:38: error: expected ')'
      REQUIRE_THROWS_AS(AGConnection conn(params), std::logic_error);
                                     ^
/Users/ruihpacheco/chi/databaseclient/cpp/tests/unit/params.cpp:22:7: note: to match this '('
      REQUIRE_THROWS_AS(AGConnection conn(params), std::logic_error);
      ^
catch-src/single_include/catch.hpp:10935:50: note: expanded from macro 'REQUIRE_THROWS_AS'
#define REQUIRE_THROWS_AS( expr, exceptionType ) INTERNAL_CATCH_THROWS_AS( expr, exceptionType, Catch::ResultDisposition::Normal, "REQUIRE_THROWS_AS" )
                                                 ^
catch-src/single_include/catch.hpp:2283:34: note: expanded from macro 'INTERNAL_CATCH_THROWS_AS'
                static_cast<void>(expr); \

2 warnings and 4 errors generated.
[24/25] Building CXX object tests/CMakeFiles/unit_tests.dir/unit/main.cpp.o
ninja: build stopped: subcommand failed.

Extra information

• Catch version: v.1.7.0
• Operating System: Latest OSX Sierra
• Compiler+version:

$ clang -v
Apple LLVM version 8.0.0 (clang-800.0.42.1)
Target: x86_64-apple-darwin16.4.0
Thread model: posix

[horenmar]

Try calling the constructor directly, without declaring a variable:
REQUIRE_THROWS_AS(AGConnection(params), std::logic_error);

The underlying problem is that to fix a -Wunused-value warning, the expr passed to REQUIRE_THROWS_AS is cast to void, which does not work when you are declaring a variable.

[jktjkt]

I came here looking for an existing bugreport about this behavior change. We're of course OK with changing our test suite to follow this upstream change, but I want to check with you that this change is intentional.

The older Catch doesn't work with the new syntax.

[horenmar]

@jktjkt Unless there is significant feedback against, we plan to keep it, so we don't trigger warnings when used like REQUIRE_THROWS_AS(static_cast<bool>(xsd::boolean("")), std::bad_cast).

The older Catch doesn't work with the new syntax.

Can you be more specific about this? I tried compiling and running this against Catch 1.6.1 using clang 3.8 and it seems to compile and work fine.

#define CATCH_CONFIG_MAIN
#include "catch.hpp"

#include <typeinfo>

struct foo {
    foo(){ throw std::bad_cast(); }
    foo(int i){ throw 1; }
};

TEST_CASE("ASD") {
        CHECK_THROWS_AS(foo(), std::bad_cast);
        CHECK_THROWS_AS(foo(3), int);
}

[jktjkt]

Previously we were on 1.5.9. I don't have a minimal example, sorry, this is a hand-sanitized snippet from our internal CI run (GCC 6.2.1 on CentOS 7, devtoolset 6). Getting rid of the variable identifier resulted in this:

xxx.cpp: In function void ____C_A_T_C_H____T_E_S_T____24():
xxx.cpp:38:43: error: no matching function for call to MyClass()
         REQUIRE_THROWS_AS(MyClass(serial), FramingError);

This is roughly how that class looks like (again, hand-sanitized):

typedef std::shared_ptr<PacketStream> StreamPtr;

class MyClass {
public:
    MyClass(StreamPtr serial);
// ...
};

Don't waste much time on this, I guess -- the change was very cheap anyway. I just wanted to provide some feedback that yeah, it was noticed :).

[ruipacheco]

Works for me.

[nicolefinnie]

Try calling the constructor directly, without declaring a variable:
REQUIRE_THROWS_AS(AGConnection(params), std::logic_error);

The underlying problem is that to fix a -Wunused-value warning, the expr passed to REQUIRE_THROWS_AS is cast to void, which does not work when you are declaring a variable.

Calling c'tor only fixed my problem as well. I was dipping my toe in the catch2 water, it's really neat and it took me only 5 minutes to get it up and running. 👍