You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by GitBox <gi...@apache.org> on 2021/04/01 16:30:25 UTC
[GitHub] [qpid-proton] DreamPearl opened a new pull request #303: [WIP] PROTON-2357: Improve test coverage in url.cpp
DreamPearl opened a new pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #303: PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303#discussion_r606148135
##########
File path: cpp/src/url_test.cpp
##########
@@ -111,6 +116,54 @@ TEST_CASE("parse URL","[url]") {
"amqp", "user", "pass", "::1", "1234", "path",
"amqp://user:pass@[::1]:1234/path");
}
+ SECTION("port") {
+ CHECK("host:1234" == url("amqp://host:1234/path").host_port());
+ CHECK(5672 == url("amqp://foo/path").port_int());
+ CHECK(5671 == url("amqps://foo/path").port_int());
+ CHECK(1234 == url("amqps://foo:1234/path").port_int());
+
+ url u("amqps://foo:xyz/path");
+ url_error caught_error("");
+ try{
+ u.port_int();
+ }
+ catch(url_error& err){
Review comment:
const?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #303: PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303#discussion_r613506969
##########
File path: cpp/src/url.cpp
##########
@@ -274,14 +274,10 @@ std::string to_string(const url& u) {
std::istream& operator>>(std::istream& i, url& u) {
std::string s;
i >> s;
- if (!i.fail() && !i.bad()) {
- if (!s.empty()) {
- url::impl* p = new url::impl(s);
- p->defaults();
- u.impl_.reset(p);
- } else {
- i.clear(std::ios::failbit);
- }
+ if (!i.fail()) {
Review comment:
Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #303: PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303#discussion_r612800966
##########
File path: cpp/src/url.cpp
##########
@@ -274,14 +274,10 @@ std::string to_string(const url& u) {
std::istream& operator>>(std::istream& i, url& u) {
std::string s;
i >> s;
- if (!i.fail() && !i.bad()) {
- if (!s.empty()) {
- url::impl* p = new url::impl(s);
- p->defaults();
- u.impl_.reset(p);
- } else {
- i.clear(std::ios::failbit);
- }
+ if (!i.fail()) {
Review comment:
```suggestion
if (i) {
```
I think that this makes the most sense. It has the same meaning as `!i.fail() && !i.bad()`, according to https://www.cplusplus.com/reference/ios/ios/operator_bool/ and it is shorter. But tbh, I never really worked with C++ streams, so I may be missing something.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #303: PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303#discussion_r606256566
##########
File path: cpp/src/url_test.cpp
##########
@@ -111,6 +116,54 @@ TEST_CASE("parse URL","[url]") {
"amqp", "user", "pass", "::1", "1234", "path",
"amqp://user:pass@[::1]:1234/path");
}
+ SECTION("port") {
+ CHECK("host:1234" == url("amqp://host:1234/path").host_port());
+ CHECK(5672 == url("amqp://foo/path").port_int());
+ CHECK(5671 == url("amqps://foo/path").port_int());
+ CHECK(1234 == url("amqps://foo:1234/path").port_int());
+
+ url u("amqps://foo:xyz/path");
+ url_error caught_error("");
+ try{
+ u.port_int();
+ }
+ catch(url_error& err){
+ caught_error=err;
+ }
+ CHECK(std::string("invalid port 'xyz'") == std::string(caught_error.what()));
+ }
+ SECTION("misc") {
+ {
+ // url copy constructor
Review comment:
Done!!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #303: PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303#discussion_r606251392
##########
File path: cpp/src/url_test.cpp
##########
@@ -111,6 +116,54 @@ TEST_CASE("parse URL","[url]") {
"amqp", "user", "pass", "::1", "1234", "path",
"amqp://user:pass@[::1]:1234/path");
}
+ SECTION("port") {
+ CHECK("host:1234" == url("amqp://host:1234/path").host_port());
+ CHECK(5672 == url("amqp://foo/path").port_int());
+ CHECK(5671 == url("amqps://foo/path").port_int());
+ CHECK(1234 == url("amqps://foo:1234/path").port_int());
+
+ url u("amqps://foo:xyz/path");
+ url_error caught_error("");
+ try{
+ u.port_int();
+ }
+ catch(url_error& err){
+ caught_error=err;
+ }
+ CHECK(std::string("invalid port 'xyz'") == std::string(caught_error.what()));
+ }
+ SECTION("misc") {
Review comment:
Thanks!! Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on pull request #303: PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303#issuecomment-819764119
Looking good. Let me merge it right now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #303: PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303#discussion_r606256771
##########
File path: cpp/src/url_test.cpp
##########
@@ -111,6 +116,54 @@ TEST_CASE("parse URL","[url]") {
"amqp", "user", "pass", "::1", "1234", "path",
"amqp://user:pass@[::1]:1234/path");
}
+ SECTION("port") {
+ CHECK("host:1234" == url("amqp://host:1234/path").host_port());
+ CHECK(5672 == url("amqp://foo/path").port_int());
+ CHECK(5671 == url("amqps://foo/path").port_int());
+ CHECK(1234 == url("amqps://foo:1234/path").port_int());
+
+ url u("amqps://foo:xyz/path");
+ url_error caught_error("");
+ try{
+ u.port_int();
+ }
+ catch(url_error& err){
+ caught_error=err;
Review comment:
Configuring clang-format would be a whole task all of its own ;( The last attempt to come up with a clang-format config for qpid-proton failed, https://github.com/apache/qpid-proton/pull/169. Sorry, this is not what you'd call actionable advice from me.
I have a feature in IDE which runs formatter only on changed lines. Then it usually does not matter that the style I set does not match the rest of the file. For cli, there is git-clang-format which should be able to do that. https://github.com/llvm-mirror/clang/blob/master/tools/clang-format/git-clang-format
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #303: PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303#discussion_r606250828
##########
File path: cpp/src/url_test.cpp
##########
@@ -111,6 +116,54 @@ TEST_CASE("parse URL","[url]") {
"amqp", "user", "pass", "::1", "1234", "path",
"amqp://user:pass@[::1]:1234/path");
}
+ SECTION("port") {
+ CHECK("host:1234" == url("amqp://host:1234/path").host_port());
+ CHECK(5672 == url("amqp://foo/path").port_int());
+ CHECK(5671 == url("amqps://foo/path").port_int());
+ CHECK(1234 == url("amqps://foo:1234/path").port_int());
+
+ url u("amqps://foo:xyz/path");
+ url_error caught_error("");
+ try{
+ u.port_int();
+ }
+ catch(url_error& err){
+ caught_error=err;
Review comment:
Thanks, Justin and Jiri for your direction. Jiri, I have installed clang-format but I am not able to configure it to match the formatting of the existing code. Can you please guide me on how to configure it according to this project?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on pull request #303: PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303#issuecomment-820392797
> Looking good. Let me merge it right now.
Thank you! :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #303: PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303#discussion_r606149268
##########
File path: cpp/src/url_test.cpp
##########
@@ -111,6 +116,54 @@ TEST_CASE("parse URL","[url]") {
"amqp", "user", "pass", "::1", "1234", "path",
"amqp://user:pass@[::1]:1234/path");
}
+ SECTION("port") {
+ CHECK("host:1234" == url("amqp://host:1234/path").host_port());
+ CHECK(5672 == url("amqp://foo/path").port_int());
+ CHECK(5671 == url("amqps://foo/path").port_int());
+ CHECK(1234 == url("amqps://foo:1234/path").port_int());
+
+ url u("amqps://foo:xyz/path");
+ url_error caught_error("");
+ try{
+ u.port_int();
+ }
+ catch(url_error& err){
+ caught_error=err;
+ }
+ CHECK(std::string("invalid port 'xyz'") == std::string(caught_error.what()));
Review comment:
You can use a Catch1 feature to check the expected exception is thrown. https://github.com/catchorg/Catch2/blob/Catch1.x/docs/assertions.md#exceptions
The feature allows you to only check exception type or exception string, but not both at the same time. To check both, you'd have to use `REQUIRE_THROWS_MATCHES` which is only available in Catch2, not Catch1.
```
CHECK_THROWS_WITH(openThePodBayDoors(), Contains( "afraid" ) && Contains( "can't do that" ));
```
Personally, I'd write two tests: first to assert that `url` throws `url_error` when it gets some completely bogus string. And then, here, I'd simply assert on the exception string and assume the exception type stayed the same...
I guess your version may actually be better than what I just proposed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #303: PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303#discussion_r606257138
##########
File path: cpp/src/url_test.cpp
##########
@@ -111,6 +116,54 @@ TEST_CASE("parse URL","[url]") {
"amqp", "user", "pass", "::1", "1234", "path",
"amqp://user:pass@[::1]:1234/path");
}
+ SECTION("port") {
+ CHECK("host:1234" == url("amqp://host:1234/path").host_port());
+ CHECK(5672 == url("amqp://foo/path").port_int());
+ CHECK(5671 == url("amqps://foo/path").port_int());
+ CHECK(1234 == url("amqps://foo:1234/path").port_int());
+
+ url u("amqps://foo:xyz/path");
+ url_error caught_error("");
+ try{
+ u.port_int();
+ }
+ catch(url_error& err){
+ caught_error=err;
+ }
+ CHECK(std::string("invalid port 'xyz'") == std::string(caught_error.what()));
+ }
+ SECTION("misc") {
+ {
+ // url copy constructor
+ url u1("amqp://foo:xyz/path");
+ url u2=u1;
+ CHECK(std::string("amqp://foo:xyz/path") == std::string(u2));
+
+ // url assignment operator
+ url u3("amqp://foo:xyz/pathdontcare");
+ u3=u1;
+ CHECK(std::string("amqp://foo:xyz/path") == std::string(u3));
+ }
+ {
+ url u("amqp://foo:1234/path");
+ CHECK(true == u.empty());
Review comment:
It looks good to me. Updated!!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] ssorj commented on a change in pull request #303: PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
ssorj commented on a change in pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303#discussion_r605897055
##########
File path: cpp/src/url_test.cpp
##########
@@ -111,6 +116,54 @@ TEST_CASE("parse URL","[url]") {
"amqp", "user", "pass", "::1", "1234", "path",
"amqp://user:pass@[::1]:1234/path");
}
+ SECTION("port") {
+ CHECK("host:1234" == url("amqp://host:1234/path").host_port());
+ CHECK(5672 == url("amqp://foo/path").port_int());
+ CHECK(5671 == url("amqps://foo/path").port_int());
+ CHECK(1234 == url("amqps://foo:1234/path").port_int());
+
+ url u("amqps://foo:xyz/path");
+ url_error caught_error("");
+ try{
+ u.port_int();
+ }
+ catch(url_error& err){
+ caught_error=err;
Review comment:
As a style and readability matter, put spaces around assignment operators
##########
File path: cpp/src/url_test.cpp
##########
@@ -111,6 +116,54 @@ TEST_CASE("parse URL","[url]") {
"amqp", "user", "pass", "::1", "1234", "path",
"amqp://user:pass@[::1]:1234/path");
}
+ SECTION("port") {
+ CHECK("host:1234" == url("amqp://host:1234/path").host_port());
+ CHECK(5672 == url("amqp://foo/path").port_int());
+ CHECK(5671 == url("amqps://foo/path").port_int());
+ CHECK(1234 == url("amqps://foo:1234/path").port_int());
+
+ url u("amqps://foo:xyz/path");
+ url_error caught_error("");
+ try{
+ u.port_int();
+ }
+ catch(url_error& err){
+ caught_error=err;
+ }
+ CHECK(std::string("invalid port 'xyz'") == std::string(caught_error.what()));
+ }
+ SECTION("misc") {
Review comment:
"Misc" is too generic here. I think "constructors" for the copy constructor case and "operators" for the remaining tests currently under misc would be more helpful to future developers.
##########
File path: cpp/src/url_test.cpp
##########
@@ -111,6 +116,54 @@ TEST_CASE("parse URL","[url]") {
"amqp", "user", "pass", "::1", "1234", "path",
"amqp://user:pass@[::1]:1234/path");
}
+ SECTION("port") {
+ CHECK("host:1234" == url("amqp://host:1234/path").host_port());
+ CHECK(5672 == url("amqp://foo/path").port_int());
+ CHECK(5671 == url("amqps://foo/path").port_int());
+ CHECK(1234 == url("amqps://foo:1234/path").port_int());
+
+ url u("amqps://foo:xyz/path");
+ url_error caught_error("");
+ try{
+ u.port_int();
+ }
+ catch(url_error& err){
+ caught_error=err;
+ }
+ CHECK(std::string("invalid port 'xyz'") == std::string(caught_error.what()));
+ }
+ SECTION("misc") {
+ {
+ // url copy constructor
+ url u1("amqp://foo:xyz/path");
+ url u2=u1;
+ CHECK(std::string("amqp://foo:xyz/path") == std::string(u2));
+
+ // url assignment operator
+ url u3("amqp://foo:xyz/pathdontcare");
+ u3=u1;
+ CHECK(std::string("amqp://foo:xyz/path") == std::string(u3));
+ }
+ {
+ url u("amqp://foo:1234/path");
+ CHECK(true == u.empty());
+ CHECK(std::string("amqp://foo:1234/path") == std::string(u));
+ CHECK(false == u.empty());
+ }
+ {
+ url u("amqp://foo:xyz/path");
+ std::ostringstream o;
+ o<<u;
Review comment:
Spaces around these too
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #303: PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303#discussion_r613104415
##########
File path: cpp/src/url.cpp
##########
@@ -274,14 +274,10 @@ std::string to_string(const url& u) {
std::istream& operator>>(std::istream& i, url& u) {
std::string s;
i >> s;
- if (!i.fail() && !i.bad()) {
- if (!s.empty()) {
- url::impl* p = new url::impl(s);
- p->defaults();
- u.impl_.reset(p);
- } else {
- i.clear(std::ios::failbit);
- }
+ if (!i.fail()) {
Review comment:
@astitcher @jiridanek I tried to investigate more into it and that leads me to think that `end-of-file sets the failbit.`
I tried to run this sample code: http://cpp.sh/3gsgh
And below are some references. Please let me know what do you think of this?
`“If no characters are extracted then std::ios::failbit is set.”`
Link: https://en.cppreference.com/w/cpp/string/basic_string/operator_ltltgtgt
`“Operations that attempt to read at the End-of-File fail, and thus both the eofbit and the failbit end up set. This function can be used to check whether the failure is due to reaching the End-of-File or to some other reason.”`
LInk: http://www.cplusplus.com/reference/ios/ios/eof/
`“Reaching the End-of-File sets the eofbit. But note that operations that reach the End-of-File may also set the failbit if this makes them fail (thus setting both eofbit and failbit).”`
Link: http://www.cplusplus.com/reference/ios/ios/fail/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #303: PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303#discussion_r612972817
##########
File path: cpp/src/url.cpp
##########
@@ -274,14 +274,10 @@ std::string to_string(const url& u) {
std::istream& operator>>(std::istream& i, url& u) {
std::string s;
i >> s;
- if (!i.fail() && !i.bad()) {
- if (!s.empty()) {
- url::impl* p = new url::impl(s);
- p->defaults();
- u.impl_.reset(p);
- } else {
- i.clear(std::ios::failbit);
- }
+ if (!i.fail()) {
Review comment:
> It has the same meaning as `!i.fail() && !i.bad()`
It does, but never mind that. `fail()` also checks the badbit, as shown in the colorful table at https://en.cppreference.com/w/cpp/io/basic_ios/fail.so what you have there now is probably best.
> it should be possible to eof and s.empty() but !fail && !bad
@DreamPearl could you investigate this more? I myself think it is not possible, but we need to be sure about this, before we make the change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #303: PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303#discussion_r606137671
##########
File path: cpp/src/url_test.cpp
##########
@@ -111,6 +116,54 @@ TEST_CASE("parse URL","[url]") {
"amqp", "user", "pass", "::1", "1234", "path",
"amqp://user:pass@[::1]:1234/path");
}
+ SECTION("port") {
+ CHECK("host:1234" == url("amqp://host:1234/path").host_port());
+ CHECK(5672 == url("amqp://foo/path").port_int());
+ CHECK(5671 == url("amqps://foo/path").port_int());
+ CHECK(1234 == url("amqps://foo:1234/path").port_int());
+
+ url u("amqps://foo:xyz/path");
+ url_error caught_error("");
+ try{
+ u.port_int();
+ }
+ catch(url_error& err){
+ caught_error=err;
+ }
+ CHECK(std::string("invalid port 'xyz'") == std::string(caught_error.what()));
+ }
+ SECTION("misc") {
+ {
+ // url copy constructor
+ url u1("amqp://foo:xyz/path");
+ url u2=u1;
+ CHECK(std::string("amqp://foo:xyz/path") == std::string(u2));
+
+ // url assignment operator
+ url u3("amqp://foo:xyz/pathdontcare");
+ u3=u1;
+ CHECK(std::string("amqp://foo:xyz/path") == std::string(u3));
+ }
+ {
+ url u("amqp://foo:1234/path");
+ CHECK(true == u.empty());
Review comment:
```suggestion
CHECK(u.empty());
```
Similarly as with `if`s, you do not have to write `== true` when checking for truthiness. But this is a matter of style. If you think that writing `== true` makes the test clearer, keep doing it. I will not complain in the future.
##########
File path: cpp/src/url_test.cpp
##########
@@ -102,6 +103,10 @@ TEST_CASE("parse URL","[url]") {
CHECK_URL(url("amqps://user%2F%3A=:pass%2F%3A=@example.net/some_topic"),
"amqps", "user/:=", "pass/:=", "example.net", "amqps", "some_topic",
"amqps://user%2F%3A=:pass%2F%3A=@example.net:amqps/some_topic");
+ // Bad Input
Review comment:
Bad how? Say it's unquoted % at the end of a percent encoded string.
##########
File path: cpp/src/url_test.cpp
##########
@@ -111,6 +116,54 @@ TEST_CASE("parse URL","[url]") {
"amqp", "user", "pass", "::1", "1234", "path",
"amqp://user:pass@[::1]:1234/path");
}
+ SECTION("port") {
+ CHECK("host:1234" == url("amqp://host:1234/path").host_port());
+ CHECK(5672 == url("amqp://foo/path").port_int());
+ CHECK(5671 == url("amqps://foo/path").port_int());
+ CHECK(1234 == url("amqps://foo:1234/path").port_int());
+
+ url u("amqps://foo:xyz/path");
+ url_error caught_error("");
+ try{
+ u.port_int();
+ }
+ catch(url_error& err){
+ caught_error=err;
+ }
+ CHECK(std::string("invalid port 'xyz'") == std::string(caught_error.what()));
+ }
+ SECTION("misc") {
+ {
+ // url copy constructor
Review comment:
Don't use braces+comment, when you can create a perfectly good SECTION
```suggestion
SECTION("url copy constructor") {
```
and so on. Here's doc on it, in case you haven't seen a Catch 1 test with nested sections before https://github.com/catchorg/Catch2/blob/Catch1.x/docs/tutorial.md#test-cases-and-sections
##########
File path: cpp/src/url_test.cpp
##########
@@ -111,6 +116,54 @@ TEST_CASE("parse URL","[url]") {
"amqp", "user", "pass", "::1", "1234", "path",
"amqp://user:pass@[::1]:1234/path");
}
+ SECTION("port") {
+ CHECK("host:1234" == url("amqp://host:1234/path").host_port());
+ CHECK(5672 == url("amqp://foo/path").port_int());
+ CHECK(5671 == url("amqps://foo/path").port_int());
+ CHECK(1234 == url("amqps://foo:1234/path").port_int());
+
+ url u("amqps://foo:xyz/path");
+ url_error caught_error("");
+ try{
+ u.port_int();
+ }
+ catch(url_error& err){
+ caught_error=err;
+ }
+ CHECK(std::string("invalid port 'xyz'") == std::string(caught_error.what()));
+ }
+ SECTION("misc") {
+ {
+ // url copy constructor
+ url u1("amqp://foo:xyz/path");
+ url u2=u1;
+ CHECK(std::string("amqp://foo:xyz/path") == std::string(u2));
Review comment:
```suggestion
CHECK(std::string(u2) == "amqp://foo:xyz/path");
```
Writing your conditions as `LITERAL == VARIABLE`, instead of the other way around, is considered a good C++ practice in some circles. (https://stackoverflow.com/questions/370366/why-put-the-constant-before-the-variable-in-a-comparison). I don't much like it, especially not in tests, and Catch 1 docs does not do it this way either, but I think it's perfectly fine if you do it here. Especially because the existing CHECK calls in this file already had this structure.
I think you don't have to do `std::string("amqp://foo:xyz/path")`, a C literal will work just as well, but I am lazy to try it. Anyways, same as before, I'd be fine if you keep it the way you have it now, it's just a thought/suggestion.
##########
File path: cpp/src/url_test.cpp
##########
@@ -111,6 +116,54 @@ TEST_CASE("parse URL","[url]") {
"amqp", "user", "pass", "::1", "1234", "path",
"amqp://user:pass@[::1]:1234/path");
}
+ SECTION("port") {
+ CHECK("host:1234" == url("amqp://host:1234/path").host_port());
+ CHECK(5672 == url("amqp://foo/path").port_int());
+ CHECK(5671 == url("amqps://foo/path").port_int());
+ CHECK(1234 == url("amqps://foo:1234/path").port_int());
+
+ url u("amqps://foo:xyz/path");
+ url_error caught_error("");
+ try{
+ u.port_int();
+ }
+ catch(url_error& err){
+ caught_error=err;
Review comment:
It is helpful to have some autoformatter for source code. Either in your IDE, or some cli tool (like clang-format). If you spend the time to configure it to match formatting of the existing code in the project, it will save you time otherwise spent on chasing spaces around equal signs.
##########
File path: cpp/src/url_test.cpp
##########
@@ -111,6 +116,54 @@ TEST_CASE("parse URL","[url]") {
"amqp", "user", "pass", "::1", "1234", "path",
"amqp://user:pass@[::1]:1234/path");
}
+ SECTION("port") {
+ CHECK("host:1234" == url("amqp://host:1234/path").host_port());
+ CHECK(5672 == url("amqp://foo/path").port_int());
+ CHECK(5671 == url("amqps://foo/path").port_int());
+ CHECK(1234 == url("amqps://foo:1234/path").port_int());
+
+ url u("amqps://foo:xyz/path");
+ url_error caught_error("");
+ try{
+ u.port_int();
+ }
+ catch(url_error& err){
+ caught_error=err;
+ }
+ CHECK(std::string("invalid port 'xyz'") == std::string(caught_error.what()));
+ }
+ SECTION("misc") {
Review comment:
Catch allows nesting the sections, so every time you feel the need to create a `{}` block, and especially a `{}` block with a comment, you can use nested `SECTION`s for each of the individual cases. See my comment below.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on pull request #303: PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303#issuecomment-812060580
@ssorj Can you please take a look and provide suggestions?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #303: PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303#discussion_r606334049
##########
File path: cpp/src/url_test.cpp
##########
@@ -111,6 +116,54 @@ TEST_CASE("parse URL","[url]") {
"amqp", "user", "pass", "::1", "1234", "path",
"amqp://user:pass@[::1]:1234/path");
}
+ SECTION("port") {
+ CHECK("host:1234" == url("amqp://host:1234/path").host_port());
+ CHECK(5672 == url("amqp://foo/path").port_int());
+ CHECK(5671 == url("amqps://foo/path").port_int());
+ CHECK(1234 == url("amqps://foo:1234/path").port_int());
+
+ url u("amqps://foo:xyz/path");
+ url_error caught_error("");
+ try{
+ u.port_int();
+ }
+ catch(url_error& err){
+ caught_error=err;
Review comment:
I have installed git-clang-format and now it's working on cli. Thanks for sharing!!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #303: PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303#discussion_r606262167
##########
File path: cpp/src/url_test.cpp
##########
@@ -111,6 +116,54 @@ TEST_CASE("parse URL","[url]") {
"amqp", "user", "pass", "::1", "1234", "path",
"amqp://user:pass@[::1]:1234/path");
}
+ SECTION("port") {
+ CHECK("host:1234" == url("amqp://host:1234/path").host_port());
+ CHECK(5672 == url("amqp://foo/path").port_int());
+ CHECK(5671 == url("amqps://foo/path").port_int());
+ CHECK(1234 == url("amqps://foo:1234/path").port_int());
+
+ url u("amqps://foo:xyz/path");
+ url_error caught_error("");
+ try{
+ u.port_int();
+ }
+ catch(url_error& err){
+ caught_error=err;
+ }
+ CHECK(std::string("invalid port 'xyz'") == std::string(caught_error.what()));
+ }
+ SECTION("misc") {
+ {
+ // url copy constructor
+ url u1("amqp://foo:xyz/path");
+ url u2=u1;
+ CHECK(std::string("amqp://foo:xyz/path") == std::string(u2));
Review comment:
Thanks for the suggestion!! LGTM :) But for maintaining consistency I kept it as `LITERAL == VARIABLE` as same as CHECK calls at the top of the file.
And yes C literals working as well. So updated that one!! Thanks!!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek merged pull request #303: PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
jiridanek merged pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on a change in pull request #303: PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on a change in pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303#discussion_r606263196
##########
File path: cpp/src/url_test.cpp
##########
@@ -111,6 +116,54 @@ TEST_CASE("parse URL","[url]") {
"amqp", "user", "pass", "::1", "1234", "path",
"amqp://user:pass@[::1]:1234/path");
}
+ SECTION("port") {
+ CHECK("host:1234" == url("amqp://host:1234/path").host_port());
+ CHECK(5672 == url("amqp://foo/path").port_int());
+ CHECK(5671 == url("amqps://foo/path").port_int());
+ CHECK(1234 == url("amqps://foo:1234/path").port_int());
+
+ url u("amqps://foo:xyz/path");
+ url_error caught_error("");
+ try{
+ u.port_int();
+ }
+ catch(url_error& err){
+ caught_error=err;
+ }
+ CHECK(std::string("invalid port 'xyz'") == std::string(caught_error.what()));
Review comment:
I went with your suggested approach as it is looking cleaner. Thanks!!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] astitcher commented on a change in pull request #303: PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
astitcher commented on a change in pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303#discussion_r613209137
##########
File path: cpp/src/url.cpp
##########
@@ -274,14 +274,10 @@ std::string to_string(const url& u) {
std::istream& operator>>(std::istream& i, url& u) {
std::string s;
i >> s;
- if (!i.fail() && !i.bad()) {
- if (!s.empty()) {
- url::impl* p = new url::impl(s);
- p->defaults();
- u.impl_.reset(p);
- } else {
- i.clear(std::ios::failbit);
- }
+ if (!i.fail()) {
Review comment:
@DreamPearl you've convinced me! Thanks for the thorough work.
I also prefer the ```i.fail()``` version
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on pull request #303: PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303#issuecomment-812563434
> Thanks, Rakhi. This looks good. I agree about the "empty" side. I think we should either drop that logic from url.cpp or change it to an assertion.
>
> I added a few small points on a per-line basis. I'll also ask one of the C++ contributors on the team to take a look.
Thanks, Justin @ssorj. Updated the url.cpp.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on pull request #303: PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303#issuecomment-812548973
> Just a quick question. Do you know how to run a single test? I hope you are not forced to run the entire testsuite every time you make a change here...
Thanks, @jiridanek to point this out as I was not aware that we can run a test for a single file though I was not running the entire testsuite, I was running tests under build/cpp. Based on your comment I found this https://github.com/apache/qpid-proton/blob/main/docs/developers.md and now managed to run a single test. Thanks!!
Now I wonder if there is any way to run make coverage for a single file too?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on pull request #303: PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303#issuecomment-812554756
> Now I wonder if there is any way to run make coverage for a single file too?
Here's the finest-grain way of running tests that is available (that I know of): Use `ctest -VV -N -R cpp-url-test` to ask ctest to print you the test command. Then add --help at the end, and you'd get a help text from Catch1:
```
$ /nix/store/33abxyajzdaggfbcrxzzpslc4582r84l-python3-3.7.9/bin/python3.7 "/home/jdanek/repos/qpid/qpid-proton/scripts/env.py" "--" "TEST_EXE_PREFIX=/nix/store/7kw3730vr14phxld7l0dm7jslkac8fly-valgrind-3.16.1/bin/valgrind --tool=memcheck --leak-check=full --error-exitcode=42 --quiet --suppressions=/home/jdanek/repos/qpid/qpid-proton/tests/valgrind.supp" "PN_SASL_CONFIG_PATH=/home/jdanek/repos/qpid/qpid-proton/cmake-build-debug/cpp/testdata/sasl-conf" "/nix/store/7kw3730vr14phxld7l0dm7jslkac8fly-valgrind-3.16.1/bin/valgrind" "--tool=memcheck" "--leak-check=full" "--error-exitcode=42" "--quiet" "--suppressions=/home/jdanek/repos/qpid/qpid-proton/tests/valgrind.supp" "/home/jdanek/repos/qpid/qpid-proton/cmake-build-debug/cpp/cpp-test" "[url]" --help
Catch v1.12.2
usage:
cpp-test [<test name, pattern or tags> ...] [options]
where options are:
-?, -h, --help display usage information
-l, --list-tests list all/matching test cases
-t, --list-tags list all/matching tags
-s, --success include successful tests in output
-b, --break break into debugger on failure
-e, --nothrow skip exception tests
-i, --invisibles show invisibles (tabs, newlines)
-o, --out <filename> output filename
-r, --reporter <name> reporter to use (defaults to console)
-n, --name <name> suite name
-a, --abort abort at first failure
-x, --abortx <no. failures> abort after x failures
-w, --warn <warning name> enable warnings
-d, --durations <yes|no> show test durations
-f, --input-file <filename> load test names to run from a file
-#, --filenames-as-tags adds a tag for the filename
-c, --section <section name> specify section to run
--list-test-names-only list all/matching test cases names only
--list-extra-info list all/matching test cases with more
info
--list-reporters list all reporters
--order <decl|lex|rand> test case order (defaults to decl)
--rng-seed <'time'|number> set a specific seed for random numbers
--force-colour force colourised output (deprecated)
--use-colour <yes|no> should output be colourised
--libidentify report name and version according to
libidentify standard
--wait-for-keypress <start|exit|both> waits for a keypress before exiting
For more detail usage please see the project docs
```
Similarly, if you run `VERBOSE=1 make coverage`, it will print you what commands it is running.
To get a clean coverage result, you have to delete the temporary files that contain previously recorded coverage, then run a test (the command from ctest), and then run the command generating the html report (from make coverage).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] jiridanek commented on a change in pull request #303: PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303#discussion_r612972817
##########
File path: cpp/src/url.cpp
##########
@@ -274,14 +274,10 @@ std::string to_string(const url& u) {
std::istream& operator>>(std::istream& i, url& u) {
std::string s;
i >> s;
- if (!i.fail() && !i.bad()) {
- if (!s.empty()) {
- url::impl* p = new url::impl(s);
- p->defaults();
- u.impl_.reset(p);
- } else {
- i.clear(std::ios::failbit);
- }
+ if (!i.fail()) {
Review comment:
> It has the same meaning as `!i.fail() && !i.bad()`
It does, but never mind that. `fail()` also checks the badbit, as shown in the colorful table at https://en.cppreference.com/w/cpp/io/basic_ios/fail. so what you have there now is probably best.
> it should be possible to eof and s.empty() but !fail && !bad
@DreamPearl could you investigate this more? I myself think it is not possible, but we need to be sure about this, before we make the change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] astitcher commented on a change in pull request #303: PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
astitcher commented on a change in pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303#discussion_r612835051
##########
File path: cpp/src/url.cpp
##########
@@ -274,14 +274,10 @@ std::string to_string(const url& u) {
std::istream& operator>>(std::istream& i, url& u) {
std::string s;
i >> s;
- if (!i.fail() && !i.bad()) {
- if (!s.empty()) {
- url::impl* p = new url::impl(s);
- p->defaults();
- u.impl_.reset(p);
- } else {
- i.clear(std::ios::failbit);
- }
+ if (!i.fail()) {
Review comment:
I think that the premise that you can't get s.empty by itself might be wrong - it looks to me that getting end-of-file is independent from fail and bad so I think it should be possible to eof and s.empty() but !fail && !bad. However I'm not any expert on iostreams either and that's just my reading of a manpage or two.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-proton] DreamPearl commented on pull request #303: [WIP] PROTON-2357: Improve test coverage in url.cpp
Posted by GitBox <gi...@apache.org>.
DreamPearl commented on pull request #303:
URL: https://github.com/apache/qpid-proton/pull/303#issuecomment-812057572
I think it's not possible to add test coverage for `i.clear(std::ios::failbit)` as if we try to make `if(!s.empty())` false then `s` has to be empty which means `if(!i.fail() && !i.bad())` will become false. Thus the targeted line can not be executed.
```cpp
std::istream& operator>>(std::istream& i, url& u) {
std::string s;
i >> s;
if (!i.fail() && !i.bad()) {
if (!s.empty()) {
url::impl* p = new url::impl(s);
p->defaults();
u.impl_.reset(p);
} else {
i.clear(std::ios::failbit); // Not covered by test
}
}
return i;
}
```
I added the following code snippet in url_test.cpp to test the targeted line but it was not getting covered under test coverage.
```cpp
{
url u("amqp://foo:xyz/path");
std::istringstream i(" ");
CHECK(false==i.fail());
i>>u;
CHECK(std::string("amqp://foo:xyz/path") == std::string(u));
CHECK(true==i.fail());
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org