[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-15 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17322151#comment-17322151
 ] 

ASF GitHub Bot commented on PROTON-2357:


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:
us...@infra.apache.org


> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Rakhi Kumari
>Priority: Major
>  Labels: starter
> Fix For: proton-c-0.35.0
>
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-14 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17321268#comment-17321268
 ] 

ASF GitHub Bot commented on PROTON-2357:


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:
us...@infra.apache.org


> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Rakhi Kumari
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-14 Thread ASF subversion and git services (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17321267#comment-17321267
 ] 

ASF subversion and git services commented on PROTON-2357:
-

Commit 17b5d30ccb3c50cc93b665729eacec2cfa64a637 in qpid-proton's branch 
refs/heads/main from Rakhi Kumari
[ https://gitbox.apache.org/repos/asf?p=qpid-proton.git;h=17b5d30 ]

PROTON-2357: Improve test coverage in url.cpp (#303)



> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Rakhi Kumari
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-14 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17321266#comment-17321266
 ] 

ASF GitHub Bot commented on PROTON-2357:


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:
us...@infra.apache.org


> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Rakhi Kumari
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-14 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17321245#comment-17321245
 ] 

ASF GitHub Bot commented on PROTON-2357:


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:
us...@infra.apache.org


> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Rakhi Kumari
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-14 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17320974#comment-17320974
 ] 

ASF GitHub Bot commented on PROTON-2357:


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:
us...@infra.apache.org


> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Rakhi Kumari
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-14 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17320863#comment-17320863
 ] 

ASF GitHub Bot commented on PROTON-2357:


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:
us...@infra.apache.org


> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-14 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17320736#comment-17320736
 ] 

ASF GitHub Bot commented on PROTON-2357:


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:
us...@infra.apache.org


> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-14 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17320735#comment-17320735
 ] 

ASF GitHub Bot commented on PROTON-2357:


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:
us...@infra.apache.org


> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-13 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17320576#comment-17320576
 ] 

ASF GitHub Bot commented on PROTON-2357:


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:
us...@infra.apache.org


> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-13 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17320551#comment-17320551
 ] 

ASF GitHub Bot commented on PROTON-2357:


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:
us...@infra.apache.org


> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-02 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17313980#comment-17313980
 ] 

ASF GitHub Bot commented on PROTON-2357:


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:
us...@infra.apache.org


> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-02 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17313914#comment-17313914
 ] 

ASF GitHub Bot commented on PROTON-2357:


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:
us...@infra.apache.org


> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-02 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17313906#comment-17313906
 ] 

ASF GitHub Bot commented on PROTON-2357:


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:
us...@infra.apache.org


> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-02 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17313905#comment-17313905
 ] 

ASF GitHub Bot commented on PROTON-2357:


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 [ ...] [options]
   
   where options are: 
 -?, -h, --help display usage information
 -l, --list-tests   list all/matching test cases
 -t, --list-tagslist all/matching tags
 -s, --success  include successful tests in output
 -b, --breakbreak into debugger on failure
 -e, --nothrow  skip exception tests
 -i, --invisibles   show invisibles (tabs, newlines)
 -o, --outoutput filename
 -r, --reporter   reporter to use (defaults to 
console)
 -n, --name   suite name
 -a, --abortabort at first failure
 -x, --abortx abort after x failures
 -w, --warn   enable warnings
 -d, --durationsshow test durations
 -f, --input-file load test names to run from a file
 -#, --filenames-as-tagsadds a tag for the filename
 -c, --sectionspecify 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 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   should output be colourised
 --libidentify  report name and version according to
libidentify standard
 --wait-for-keypress   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:
us...@infra.apache.org


> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make 

[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-02 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17313904#comment-17313904
 ] 

ASF GitHub Bot commented on PROTON-2357:


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:
us...@infra.apache.org


> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-02 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17313900#comment-17313900
 ] 

ASF GitHub Bot commented on PROTON-2357:


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:
us...@infra.apache.org


> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-02 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17313899#comment-17313899
 ] 

ASF GitHub Bot commented on PROTON-2357:


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:
us...@infra.apache.org


> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-02 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17313898#comment-17313898
 ] 

ASF GitHub Bot commented on PROTON-2357:


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:
us...@infra.apache.org


> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-02 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17313896#comment-17313896
 ] 

ASF GitHub Bot commented on PROTON-2357:


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:
us...@infra.apache.org


> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-02 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17313891#comment-17313891
 ] 

ASF GitHub Bot commented on PROTON-2357:


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:
us...@infra.apache.org


> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-02 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17313889#comment-17313889
 ] 

ASF GitHub Bot commented on PROTON-2357:


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:
us...@infra.apache.org


> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-02 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17313723#comment-17313723
 ] 

ASF GitHub Bot commented on PROTON-2357:


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:
us...@infra.apache.org


> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-02 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17313722#comment-17313722
 ] 

ASF GitHub Bot commented on PROTON-2357:


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:
us...@infra.apache.org


> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-02 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17313712#comment-17313712
 ] 

ASF GitHub Bot commented on PROTON-2357:


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. 

[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-01 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17313436#comment-17313436
 ] 

ASF GitHub Bot commented on PROTON-2357:


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< [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-01 Thread Justin Ross (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17313429#comment-17313429
 ] 

Justin Ross commented on PROTON-2357:
-

It also seems that the check for bad() is redundant, since fail() already 
checks that:

http://www.cplusplus.com/reference/ios/ios/fail/

> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-01 Thread Justin Ross (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17313428#comment-17313428
 ] 

Justin Ross commented on PROTON-2357:
-

I think I agree about the empty side of that branch.  I don't see how it can be 
triggered.

> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-01 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17313339#comment-17313339
 ] 

ASF GitHub Bot commented on PROTON-2357:


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:
us...@infra.apache.org


> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-01 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17313337#comment-17313337
 ] 

ASF GitHub Bot commented on PROTON-2357:


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:
us...@infra.apache.org


> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-01 Thread ASF GitHub Bot (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17313292#comment-17313292
 ] 

ASF GitHub Bot commented on PROTON-2357:


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:
us...@infra.apache.org


> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>  Labels: starter
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-01 Thread Rakhi Kumari (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17313252#comment-17313252
 ] 

Rakhi Kumari commented on PROTON-2357:
--

Thanks for the suggestion!! I'm on it.

> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-01 Thread Justin Ross (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17313250#comment-17313250
 ] 

Justin Ross commented on PROTON-2357:
-

[~DreamPearl], if you are already at 98%, I think raising a PR for initial 
review is a good idea.  You'll likely get comments about how to structure the 
tests and that kind of thing.

> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>
> *Assignee: Rakhi Kumari*
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-01 Thread Justin Ross (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17313242#comment-17313242
 ] 

Justin Ross commented on PROTON-2357:
-

Hi, [~sushmau].  I assigned this one to Rakhi.  I'll find another task for you.

> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-01 Thread Rakhi Kumari (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17313212#comment-17313212
 ] 

Rakhi Kumari commented on PROTON-2357:
--

Hi [~sushmau] [~jross]
 Looks like there has been some confusion, I thought I had to work on this bug 
as per the conversation on the mailing list, and I am almost done and got 98% 
coverage already.

Justin, can you please take a look?

> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp

2021-04-01 Thread Sushma Unnibhavi (Jira)


[ 
https://issues.apache.org/jira/browse/PROTON-2357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17313183#comment-17313183
 ] 

Sushma Unnibhavi commented on PROTON-2357:
--

I have started working on this issue and will submit a patch when I finish this.

> [cpp] Improve test coverage in url.cpp
> --
>
> Key: PROTON-2357
> URL: https://issues.apache.org/jira/browse/PROTON-2357
> Project: Qpid Proton
>  Issue Type: Test
>  Components: cpp-binding
>Reporter: Justin Ross
>Assignee: Justin Ross
>Priority: Major
>
> Url.cpp currently has 76% line coverage after running the tests.  Increase 
> the coverage to 100% or as close as is practical.  To do this, add or modify 
> tests in url_test.cpp.
> To set up coverage builds:
>  # Install lcov, the code coverage tool
>  # Configure the build for coverage analysis: cmake 
> -DCMAKE_BUILD_TYPE=Coverage [...]
>  # Build the code: make build
>  # Run the tests: make test
>  # Generate coverage results: make coverage
>  # View the results at /coverage_results



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org