[jira] [Commented] (PROTON-2357) [cpp] Improve test coverage in url.cpp
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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
[ 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