You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2022/04/28 09:57:14 UTC
[GitHub] [nifi-minifi-cpp] lordgamez opened a new pull request, #1320: MINIFICPP-1813 Stabilize AbsoluteTimeoutTest in CI on Windows
lordgamez opened a new pull request, #1320:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1320
Due to scheduling issues when too many tests are run parallel the read timeout limit can be reached before we reach the absolute timeout (3 times the read timeout).
https://issues.apache.org/jira/browse/MINIFICPP-1813
------------------------------------------------------
Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [ ] Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [ ] Has your PR been rebased against the latest commit within the target branch (typically main)?
- [ ] Is your initial contribution a single, squashed commit?
### For code changes:
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the LICENSE file?
- [ ] If applicable, have you updated the NOTICE file?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
### Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] martinzink closed pull request #1320: MINIFICPP-1813 Stabilize AbsoluteTimeoutTest in CI on Windows
Posted by GitBox <gi...@apache.org>.
martinzink closed pull request #1320: MINIFICPP-1813 Stabilize AbsoluteTimeoutTest in CI on Windows
URL: https://github.com/apache/nifi-minifi-cpp/pull/1320
--
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.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] lordgamez commented on a diff in pull request #1320: MINIFICPP-1813 Stabilize AbsoluteTimeoutTest in CI on Windows
Posted by GitBox <gi...@apache.org>.
lordgamez commented on code in PR #1320:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1320#discussion_r861545182
##########
extensions/http-curl/tests/AbsoluteTimeoutTest.cpp:
##########
@@ -22,20 +22,14 @@
#include "tests/TestServer.h"
#include "HTTPHandlers.h"
+using namespace std::literals::chrono_literals;
+
int main() {
TestController controller;
std::string port = "12324";
std::string rootURI = "/";
- TimeoutingHTTPHandler handler({
- std::chrono::milliseconds(500),
- std::chrono::milliseconds(500),
- std::chrono::milliseconds(500),
- std::chrono::milliseconds(500),
- std::chrono::milliseconds(500),
- std::chrono::milliseconds(500),
- std::chrono::milliseconds(500)
- });
+ TimeoutingHTTPHandler handler({35, 100ms});
Review Comment:
You are right, it's more explicit and clear this way, updated in 8497f10ca144a7d8053d83ae67c24942168e4540
--
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.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] szaszm commented on a diff in pull request #1320: MINIFICPP-1813 Stabilize AbsoluteTimeoutTest in CI on Windows
Posted by GitBox <gi...@apache.org>.
szaszm commented on code in PR #1320:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1320#discussion_r860797382
##########
extensions/http-curl/tests/AbsoluteTimeoutTest.cpp:
##########
@@ -22,20 +22,14 @@
#include "tests/TestServer.h"
#include "HTTPHandlers.h"
+using namespace std::literals::chrono_literals;
+
int main() {
TestController controller;
std::string port = "12324";
std::string rootURI = "/";
- TimeoutingHTTPHandler handler({
- std::chrono::milliseconds(500),
- std::chrono::milliseconds(500),
- std::chrono::milliseconds(500),
- std::chrono::milliseconds(500),
- std::chrono::milliseconds(500),
- std::chrono::milliseconds(500),
- std::chrono::milliseconds(500)
- });
+ TimeoutingHTTPHandler handler({35, 100ms});
Review Comment:
I would change this to have explicit type with parens instead of braces. Usually I prefer this style, but in this case, I want to make it clear that this is not the initializer_list contstructor of std::vector, but it's going to create 35 elements with the value 100ms. I got bitten by a similar thing when I tried to direct-list-initialize a string with N elements of a certain character, but it got converted to char and used the initializer_list constructor instead of the two parameter one. After changing to parens, it used the two parameter ctor.
```suggestion
TimeoutingHTTPHandler handler(std::vector<std::chrono::milliseconds>(35, 100ms));
```
--
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.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org